Ogg playback differs between LMMS versions

[tresf]

This issue was originally reported in #dev-casual on Discord.

• drums/crash01.ogg
  • Most versions of LMMS load this file as 923 ms
  • Some versions of LMMS load this file as 1311 ms

Some baselines:

• ffprobe drums/crash.ogg shows 1311 ms
• Audacity on macOS shows 1311 ms
• Audacity on Windows shows 923 ms
• LMMS on macOS shows 923 ms

According to this article, the files are most likely at fault, quoting:

[length of OGG Vorbis is] an issue with your file. Also, I've reproduced the issue in VLC [...]

If this is true, this should be a way to detect faulty files:

find ~/lmms/data -type f -name "*.ogg" -print0 | xargs -0 -I {} bash -c 'oggz validate "{}" >/dev/null 2>&1 || echo "Validation failed for: {}"' |sort

Notice crash01.ogg appears here:

Validation failed for: ~/lmms/data/samples/drums/bassdrum01.ogg
Validation failed for: ~/lmms/data/samples/drums/bassdrum04.ogg
Validation failed for: ~/lmms/data/samples/drums/clap01.ogg
Validation failed for: ~/lmms/data/samples/drums/crash01.ogg
Validation failed for: ~/lmms/data/samples/drums/hihat_opened02.ogg
Validation failed for: ~/lmms/data/samples/drums/kick_hardcore01.ogg
Validation failed for: ~/lmms/data/samples/drums/shaker01.ogg
Validation failed for: ~/lmms/data/samples/drums/snare01.ogg
Validation failed for: ~/lmms/data/samples/instruments/cello01.ogg
Validation failed for: ~/lmms/data/samples/instruments/e_organ01.ogg
Validation failed for: ~/lmms/data/samples/misc/applause01.ogg

We need to find a way to render these .ogg files as they've HISTORICALLY sounded like (e.g. using Audacity on Windows) so that projects don't change sound as a result of a library update.

Tagging @StakeoutPunch @Monospace-V.

[Monospace-V]

I find a discrepancy between the ffprobe listed length and the Audacity-shown length for three of the listed files. Assuming Audacity behaves the way LMMS does for the ogg files, these files seem to have issue:

/drums/crash01.ogg
/instruments/cello01.ogg
/instruments/e-organ01.ogg

e-organ01 sounds like a vehicle horn, so it may not necessarily seem to need fixing upon first sight, but the length of the file could cause compatibility issues.

[tresf]

Pinging @StakeoutPunch again, I think this is a rather quick to fix bug.

[Monospace-V]

It has been observed in 0.4.15, 1.1.2, and 1.1.3 (the last of which has been observed by another user) that the sample length of crash01.ogg is actually extended at 1311 ms.
This will not be a simple replacement fix. It is starting to look as though something was changed in 1.2's ogg behaviour which was never noticed.

[tresf]

It has been observed in 0.4.15, 1.1.2, and 1.1.3 (the last of which has been observed by another user) that the sample length of crash01.ogg is actually extended at 1311 ms.

Whatever's most common decoding/playback behavior historically is what we should aim to preserve.

It is starting to look as though something was changed in 1.2's ogg behaviour which was never noticed.

The decoders will have slight changes overtime. Due to how sparse this problem is, I believe this change to be how the decoder handles a bad file. We only need to preserve the most common behavior here. Whatever that is, we fix the OGGs to match and move on from this bug. This should be as simple as re-exporting the OGGs in the "expected" format, nothing more. If you're not comfortable with creating the PR, I'd be happy to do so and add accreditation in the logs. I don't think I can create these OGGs because I don't have a PC capable of reproducing the "old" behavior, but I'm happy to do the rest.

[sakertooth]

Whatever's most common decoding/playback behavior historically is what we should aim to preserve.

I disagree in general. I understand potential backward compatibility concerns, but we should aim for correct behavior. soxi and ffprobe all show 1.31s, so 1311ms should be the correct length that we need to restore.

[sakertooth]

Example, historically, LMMS potentially drops input audio data when using libsamplerate because it doesnt always check how many were used and how many were generated. Fixing this can ultimately change the sound, but in no way do I want to preserve this kind of wack behavior.

[tresf]

Whatever's most common decoding/playback behavior historically is what we should aim to preserve.

I disagree in general. I understand potential backward compatibility concerns, but we should aim for correct behavior. soxi and ffprobe all show 1.31s, so 1311ms should be the correct length that we need to restore.

You can disagree, but it won't change our direction unless someone provides concrete reasoning. So far, this is a devil's advocate argument. In the SPECIFIC case of this bug, it was evaluated on Discord and it's just fine. We're not preserving shitty behavior, we're preserving historical, sane behavior.

Example, historically, LMMS potentially drops input audio data when using libsamplerate because it doesn't always check how many were used and how many were generated. Fixing this can ultimately change the sound, but in no way do I want to preserve this kind of wack behavior.

Please do not fight a cause with indeterministic hypotheticals. This bug went unnoticed for a decade because it wasn't THAT bad of a bug. If someone wants to fight for pragmatic reasons, use solid reasoning. This should NOT be compared to sporadic dropping input audio data. This is a reproducible change in decoding behavior for a handful of files. A decision must be made and the action to fix these should be quick, accepted and the bug is closed. There's no reason to drag this out.

[tresf]

Whatever's most common decoding/playback behavior historically is what we should aim to preserve.

I disagree in general.

@sakertooth and I discussed this in Discord and what I believe he's saying is to not take my statement as gospel for the project as a whole. If that's the case, I agree. ❤

I generally favor "fixing" bugs over "preserving" bugs (and we all should), but this is just an exception since only a few samples are affected. I think preserving the way they've historically sounded is the sane path forward.