Fix 44.1k export resampling and add 16-bit TPDF dithering#4
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe changes integrate audio resampling into the export workflow, add triangular dithering for 16-bit WAV encoding, and implement safer bit-depth handling during audio encoding. A new resampleAudioBuffer function is introduced and called conditionally during export if sample rates mismatch. Changes
Sequence DiagramsequenceDiagram
participant App as app.js<br/>(Export Flow)
participant Worker as Worker<br/>(Render)
participant Renderer as renderer.js<br/>(Resampling)
participant Encoder as encoder.js<br/>(WAV Encoding)
participant Output as Output<br/>(WAV Blob)
App->>Worker: Render with target sample rate
Worker-->>App: Return audioBuffer
App->>App: Check if sample rates match
alt Sample rate mismatch
App->>Renderer: resampleAudioBuffer(buffer, targetRate)
Renderer->>Renderer: Create OfflineAudioContext
Renderer-->>App: Return resampled buffer
App->>App: Update progress to 83%
end
App->>App: Guard: Check for cancellation
App->>Encoder: encodeWAV(resampledBuffer, bitDepth)
Encoder->>Encoder: Apply triangular dither (16-bit)
Encoder->>Encoder: Clamp samples to target bit-depth range
Encoder-->>App: Return encoded WAV data
App->>Output: Create Blob from WAV data
Output-->>App: Export complete
Estimated Code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍(Compliance updated until commit 30a1bd1)Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label Previous compliance checksCompliance check up to commit 88c8cde
|
|||||||||||||||||||||||||||||||||||||||||||||||||
PR Code Suggestions ✨Latest suggestions up to 30a1bd1
Previous suggestions✅ Suggestions up to commit 88c8cde
|
|||||||||||||||||||||||||||
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@web/app.js`:
- Around line 1422-1438: The progress bar regresses from 85% to 83% during
resampling; update the resampling progress call so it never goes below the
current encoding start point. Replace the updateProgress(83, ...) call used
before calling resampleAudioBuffer(exportBuffer, parsedSampleRate) with a value
>=85 (e.g. updateProgress(86, `Resampling to ${parsedSampleRate / 1000}kHz...`)
or compute a value between 85 and the encoding start) so progress is monotonic;
keep using the same symbols exportBuffer, parsedSampleRate, updateProgress, and
resampleAudioBuffer.
Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com>
Co-authored-by: qodo-code-review[bot] <151058649+qodo-code-review[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
User description
Summary
Fixes incorrect playback speed/duration when exporting to 44.1 kHz and adds proper 16-bit WAV dithering.
Problem
Exporting at
44.1 kHzcould produce a WAV that played too fast (for example, ~5:24source becoming ~5:00).Root Cause
In the worker export path, audio was rendered at the source sample rate (commonly
48 kHz) but encoded with a44.1 kHzWAV header without resampling first.What Changed
Added sample-rate conversion before worker-path encoding:
resampleAudioBuffer(...)inweb/ui/renderer.jsencodeWAVAsync(...)inweb/app.jsAdded built-in 16-bit TPDF dithering (no FFmpeg required):
encodeWAV(...)andencodeWAVAsync(...)inweb/ui/encoder.jsEncoder robustness updates:
safeBitDepthsafeBitDepth[-32768, 32767]Impact
Validation
npm run build:webpasses.Manual Verification
PR Type
Bug fix, Enhancement
Description
Fix 44.1 kHz export playback speed by resampling before encoding
Add 16-bit TPDF dithering to reduce quantization noise artifacts
Normalize bit-depth handling with safeBitDepth validation
Add post-dither clamping to prevent 16-bit overflow
Diagram Walkthrough
File Walkthrough
app.js
Add resampling before worker export encodingweb/app.js
resampleAudioBufferhelper function from rendererencodeWAVAsyncinstead of originalencoder.js
Add TPDF dithering and normalize bit-depth handlingweb/ui/encoder.js
triangularDither()function for TPDF dithering in LSB domainsafeBitDepthvariable (defaults to16)
encodeWAVandencodeWAVAsync[-32768, 32767]range for 16-bit samplessafeBitDepthinstead of rawbitDepthrenderer.js
Add audio buffer resampling utility functionweb/ui/renderer.js
resampleAudioBuffer()export function usingOfflineAudioContext
calculation
Summary by CodeRabbit