hendriks73 / ffsampledsp

FFmpeg based service provider for javax.sound.sampled.
GNU Lesser General Public License v2.1
24 stars 5 forks source link

Incorrect value for FFAudioFileFormat#getFrameLength() #16

Closed MrGazdag closed 2 years ago

MrGazdag commented 2 years ago

Hi, I have encountered an error in FFSampledSP. I tried to query the length of the audio file i'm working with in frames, however, i received a value of 2519. This is definitely wrong, as the audio i'm testing with is a 48000Hz ~60 second mp3 file. Using the following code, you can reproduce the bug:

File file = new File("test.mp3");
AudioFileFormat fileFormat = AudioSystem.getAudioFileFormat(file); //This is definitely an FFAudioFileFormat class
long expectedFrameLength = (long) (((long)fileFormat.getProperty("duration")) * fileFormat.getFormat().getSampleRate() / 1000 / 1000);
long actualFrameLength = fileFormat.getFrameLength();
System.out.println("Expected Frame Length: " + expectedFrameLength);
System.out.println("Actual Frame Length: " + actualFrameLength);

The output of this code should be the following:

Expected Frame Length: 2901888
Actual Frame Length: 2519

You can use the mp3 file in the following archive to reproduce these results: test.zip

The problem originates from native sources, as in the constructor of FFAudioFileFormat (which is called by native code), the frameLength parameter is already wrong. The value of frameLength gets calculated in FFAudioFileReader.c. I could not debug the native code, therefore I do not know the main source of this bug.

I am using the latest version of FFSampledSP on maven central (which is currently 0.9.46).

hendriks73 commented 2 years ago

When decoding an mp3 file, FFSampledSP correctly gives you the number of mp3 frames, not samples. Unfortunately, Java's documentation is a little misleading.

Please see https://bugs.openjdk.org/browse/JDK-8279338 for details.

MrGazdag commented 2 years ago

When decoding an mp3 file, FFSampledSP correctly gives you the number of mp3 frames, not samples.

Indeed, this is what I’ve expected, I just could not verify that the number was the amount of mp3 frames.

Unfortunately, Java's documentation is a little misleading.

I would say it defines the measurement unit clearly. If the getFrameLength documentation explicitly states "sample frames", I expect the return value to be equal to the amount of frames of the same audio in an uncompressed format. Could this be changed? Does returning the amount of mp3 frames even have a use on the Java side?

hendriks73 commented 2 years ago

I would say it defines the measurement unit clearly. If the getFrameLength documentation explicitly states "sample frames", I expect the return value to be equal to the amount of frames of the same audio in an uncompressed format.

You are expecting something the API is not promising.

Could this be changed? Does returning the amount of mp3 frames even have a use on the Java side?

This will not be changed, as it would break the API. The reasoning is very clearly explained in https://bugs.openjdk.org/browse/JDK-8279338 Essentially, "sample frames" is an undefined term, therefore it cannot be explicit. The paragraph in the Java tutorial explains what is to be expected here for compressed formats. FFSampledSP adheres to that.

MrGazdag commented 2 years ago

I see, for some reason my eyes decided to skip the part of the quote in the bug report where it describes frames for MP3. Sorry for the inconvenience, it seems I will have to use the calculation above to calculate the expected PCM frame length.

MrGazdag commented 2 years ago

By the way, thank you very much for the quick reply.