Incorrect MP3 duration

[naglis]

Reproducer

To reproduce, I am using a file filled with 10 minutes (600s) silence generated by ffmpeg using this command (the file is also attached):

ffmpeg -f lavfi -i anullsrc=r=11025:cl=mono -t 600 -acodec mp3 silence.mp3

ffprobe on the generated file says the duration is 600.137143 seconds:

ffprobe -hide_banner -show_format silence.mp3
Input #0, mp3, from 'silence.mp3':
  Metadata:
    encoder         : Lavf61.1.100
  Duration: 00:10:00.14, start: 0.100227, bitrate: 16 kb/s
  Stream #0:0: Audio: mp3 (mp3float), 11025 Hz, mono, fltp, 16 kb/s
[FORMAT]
filename=silence.mp3
nb_streams=1
nb_programs=0
nb_stream_groups=0
format_name=mp3
format_long_name=MP2/3 (MPEG audio layer 2/3)
start_time=0.100227
duration=600.137143
size=1200526
bit_rate=16003
probe_score=51
TAG:encoder=Lavf61.1.100
[/FORMAT]

However, running lofty's tag_reader example from dde24d5 prints:

cargo run --example tag_reader silence.mp3
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.05s
     Running `target/debug/examples/tag_reader silence.mp3`
--- Tag Information ---
Title: None
Artist: None
Album: None
Genre: None
Album Artist: None
--- Audio Properties ---
Bitrate (Audio): 16
Bitrate (Overall): 16
Sample Rate: 11025
Bit depth: 0
Channels: 1
Duration: 09:57

which is off by 3 seconds.

After digging in the code I believe it is because here we lose the fractional part from frame_time, since it is computed separately as an integer before multiplying by the number of frames.

If I change the length computation to multiply by the number of frames before dividing by sample rate:

diff --git a/lofty/src/mpeg/properties.rs b/lofty/src/mpeg/properties.rs
index c703a94..ab5dc97 100644
--- a/lofty/src/mpeg/properties.rs
+++ b/lofty/src/mpeg/properties.rs
@@ -152,9 +152,9 @@ where
 
 	if let Some(xing_header) = xing_header {
 		if first_frame_header.sample_rate > 0 && xing_header.is_valid() {
-			let frame_time = (u32::from(first_frame_header.samples) * 1000)
-				.div_round(first_frame_header.sample_rate);
-			let length = u64::from(frame_time) * u64::from(xing_header.frames);
+			let length = (u64::from(first_frame_header.samples)
+				* 1000 * u64::from(xing_header.frames))
+			.div_round(u64::from(first_frame_header.sample_rate));
 
 			properties.duration = Duration::from_millis(length);
 			properties.overall_bitrate = ((file_length * 8) / length) as u32;

I get the correct duration:

cargo run --example tag_reader silence.mp3
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.07s
     Running `target/debug/examples/tag_reader silence.mp3`
--- Tag Information ---
Title: None
Artist: None
Album: None
Genre: None
Album Artist: None
--- Audio Properties ---
Bitrate (Audio): 16
Bitrate (Overall): 16
Sample Rate: 11025
Bit depth: 0
Channels: 1
Duration: 10:00

Summary

First of all, I'd like to thank you for this library. I really appreciate all the work that you put into it.

I am using it in context of audiobooks where single-file audiobooks can be quite long, and have noticed that the duration of MP3 files returned by lofty was incorrect (when comparing to the duration returned by other tools, like ffmpeg). This led to some weird results when the audiobook playback position was seemingly greater than the length of the audiobook and where a chapter was supposed to start after the audiobook had already ended. FWIW, from my limited testing with MP4/FLAC/OPUS/OGG, all those file formats returned the correct duration.

E.g. for an MP3 file of 14.5 hours, the duration returned by lofty would be ~4 minutes shorter.

Expected behavior

The duration of MP3 files of known duration returned by lofty should be correct and match the duration returned by other libraries/tools.

Assets

silence.zip

[Serial-ATA]

Thanks for the super detailed report!

Your fix works nicely. While testing this, I found out that files without VBR headers have durations that are slightly off as well. I'll look into that tomorrow and get this fixed in 0.20.1.