Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix zeros in the first m4a chunk on Linux #11879

Merged
merged 1 commit into from
Aug 28, 2023
Merged

Conversation

daschuer
Copy link
Member

This starts decoding of the first frame with the required preroll, going to < 1.
Before the reading of the peroll in this region has been skipped (clamped to zero).
Now the FFmpeg soundsource is head up with CoreAudio

Red Line before, Blue line after.

grafik

This fixes #11878

The issue has been discovered by a SoundSourceProxyTest.firstSoundTest in my pipeline.

Unfortunately this fix required a waveform re-analyzis if the first sound starts in this region (0 ... 23 ms)
This fix does not introduce an offset so that cue points are not shiftet.

@daschuer daschuer added this to the 2.4.0 milestone Aug 28, 2023
@daschuer
Copy link
Member Author

The SoundSourceProxyTest.firstSoundTest is here in action:
https://github.com/daschuer/mixxx/actions/runs/5996017702

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

I'm anything but an expert on audio decoding, but this LGTM.

Copy link
Contributor

@uklotzde uklotzde left a comment

Choose a reason for hiding this comment

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

Good catch.

// Seek to codec frame boundaries if the frame size is fixed and known
if (m_pavStream->codecpar->frame_size > 0) {
seekIndex -= seekIndex % m_pavCodecContext->frame_size;
}
DEBUG_ASSERT(seekIndex >= 0);
DEBUG_ASSERT(seekIndex <= startIndex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping the second debug assertion wouldn't hurt.

@Swiftb0y
Copy link
Member

Please readd the second debug assert, then I'll merge

@daschuer
Copy link
Member Author

Rebased it back in.

@Swiftb0y Swiftb0y merged commit faae539 into mixxxdj:2.4 Aug 28, 2023
10 checks passed
@daschuer daschuer deleted the gh11878 branch September 6, 2023 05:38
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.

3 participants