Skip to content

fix: handle non-16kHz audio correctly in VideoClipper - #218

Open
Minh3132 wants to merge 3 commits into
modelscope:mainfrom
Minh3132:fix/non-16khz-audio-214
Open

Minh3132 wants to merge 3 commits into
modelscope:mainfrom
Minh3132:fix/non-16khz-audio-214

Conversation

@Minh3132

@Minh3132 Minh3132 commented Sep 14, 2026

Copy link
Copy Markdown

Summary

Fix non-16kHz audio handling in VideoClipper.

Previously, audio could be resampled before the channel layout and effective sample rate were handled consistently. This could cause incorrect behavior when processing audio whose original sample rate was not 16 kHz, especially for stereo inputs.

Changes

  • Handle the audio channel before resampling.
  • Resample non-16kHz input audio to 16 kHz consistently.
  • Keep the effective sample rate consistent with the resampled waveform.
  • Preserve existing behavior for 16 kHz input.
  • Add regression tests covering multiple sample rates, channel layouts, and dtypes.

Validation

Regression coverage includes:

  • 8 kHz
  • 16 kHz
  • 44.1 kHz
  • 48 kHz
  • mono and stereo inputs
  • float32 and int16 inputs
  • input immutability
  • output sample-rate handling
  • clip duration / waveform behavior

The regression tests are designed to verify the non-16kHz audio handling fix across these combinations.

This description does not claim full end-to-end UI, browser, ASR-model, or video-export validation.

Fixes #214

@LauraGPT LauraGPT left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for separating #214's normalization fix from text matching. I checked exact head eabeee4 with the candidate VideoClipper module and five base helper files verified against main 9e72061 Git blob hashes.

The submitted normalization test plus existing recognition-result and duplicate-matching tests pass: 15/15. Independently, 16 combinations of 8/16/44.1/48 kHz, mono/stereo, and float32/int16 all preserve the caller's input, send the expected first-channel 16,000 samples for one second to the ASR capture stand-in, store 16 kHz, and return 8,000 samples at 16 kHz for a 0.5-second explicit clip. Waveforms match a separate first-channel librosa resampling reference. On the base implementation only the four already-16-kHz controls satisfy these assertions; the other 12 fail the rate/sample-count contract.

This is real librosa and actual recog/clip code with a capture stand-in at model.generate, not real model inference, browser, video export, a full repository suite or an accuracy/performance claim. An initial auxiliary 0.1-0.3-second clip fixture hit the base subtitle helper's existing empty-timestamp IndexError; I retained that failure and used a fully covered 0-0.5-second timestamp for the normalization comparison. This PR does not claim to fix that subtitle edge case.

Please replace the untouched TODO/template body with the actual scope, relation to #214 and commands/results, and consider committing the mono/already-16k/44.1k/8k/int16 and first-channel waveform controls so this evidence remains reproducible in the repository. Keep #215/#216 and the speaker-toggle issue out of this change.

@Minh3132 Minh3132 changed the title Fix/non 16khz audio 214 fix: handle non-16kHz audio correctly in VideoClipper Sep 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Non-16 kHz audio is resampled on the channel axis and exported with a stale sample rate

2 participants