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

Analysis waveform drifts when exporting metadata after analyzing an MP3 file with ID3 v1.1 tags #11159

Closed
robbert-vdh opened this issue Dec 28, 2022 · 10 comments

Comments

@robbert-vdh
Copy link
Contributor

Bug Description

See the Zulip chat for more context: https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/Analysis.20waveform.20sometimes.20drifting.20slightly This one is super specific and it took a while to figure out a pattern. I'm still not entirely sure about why what's happening is causing problems, but it seemed like a good idea to at least submit a bug report for this now that we have a lead.

The problem is that when analyzing certain MP3 files and then saving the metadata to file tags (either because the relevant option in Settings -> Library is enabled or through Right click -> Metadata -> Export to file tags, this may require restarting Mixxx if the export is deferred), the analyzed waveform is incorrect. The waveform usually drifts to the left, although I've also seen the same thing happen with waveforms drifting to the right. This makes it look like the tempo is slightly off as the beat grid doesn't align with the waveform, even though the beat grid is actually correct. Using the scratching controls with a controller or the waveform display to focus on a transient will confirm that the beat grid is indeed correct and that the waveform display is off. If you then go to Right click -> Reset -> Waveform and then analyze the file again the waveform will be correct.

I had a hard time figuring out a pattern, but I just noticed that this only seems to happen with MP3 files containing ID3v1.1 tags in addition to ID3v2.4.0 tags. MP3 files containing just those ID3v2.4.0 tags don't have the issue.

This .zip archive contains two MP3 files: mp3-test-files.zip think-break-with-ID3-tags.mp3 is unaffected by the issue. think-break-with-additional-ID3-v1.1-tags.mp3 is the exact same file, except for some additional ID3v1.1 tags added using kid3.

Version

main branch, 86a280b

OS

Arch Linux

@uklotzde
Copy link
Contributor

Caused by a missing continue; statement that leads to decoding audio samples from non-audio MP3 frames. Fixing this bug will change all audio positions of MP3 files that contain metadata before the actual audio data.

Getting rid of SoundSourceMP3 and instead replacing it with SoundSourceFFmpeg would be a safer and more sustainable solution in the long term.

@uklotzde
Copy link
Contributor

This issue affects all MP3 files that contain any MP3 metadata frames (ID3v1/ID3v2/APE) before the MP3 audio frames.

@daschuer
Copy link
Member

daschuer commented Dec 30, 2022

I cannot confirm your assumptions. I am currently working on a solution with SoundSourceMp3.

@uklotzde
Copy link
Contributor

I cannot confirm your assumptions. I am currently working on a solution with SoundSourceMp3.

The fix is easy and obvious but it has serious implications. Nevertheless, the current situation is also bad.

@robbert-vdh
Copy link
Contributor Author

Caused by a missing continue; statement that leads to decoding audio samples from non-audio MP3 frames.

Great find! I was confident that something like this was the case considering the circumstances, but I hadn't looked at any code yet. If I don't forget to I'll try to force SoundSourceFFmpeg and check if that makes any difference.

@uklotzde
Copy link
Contributor

The fix seems to me more complicated, tests are failing. Nevertheless, the situation and its implications are as bad as they could be. The length and offset of the decoded audio when using SoundSourceMP3 currently depends on the presence/absence of tags.

@daschuer
Copy link
Member

Only the length changes on presence/absence of tags.

The issue of skipping the decoding of the first frame has been introduced in SoundSourceMP3 in Mixxx 2.1.0 2015 #411 a fix workaround for non accurate seeks compared to start the track from the very beginning.

@daschuer
Copy link
Member

Probably skipping the first frame is correct. I have recorded a wav file and a mp3 file with Mixxx the wav file starts immediately while the mp3 file has 20 ms noise before the signal starts. The skipped frame is completely silence.
It is a Lame Info Frame:
https://stackoverflow.com/questions/5710598/is-the-leading-structure-in-mp3-file-a-real-frame

The issue might be that this frame is skipped unconditionally in Mixxx. Audacity decodes the frame as audio.

@daschuer
Copy link
Member

daschuer commented Jan 1, 2023

#11168 is done now please have a look.

@robbert-vdh
Copy link
Contributor Author

I gave #11168 a brief try and it seems to fix the issue. Haven't done any extensive testing nor have I looked at the patch though.

@daschuer daschuer added this to the 2.3.5 milestone Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants