Error reading tag from mp3 file

[milesegan]

Reproducer

I tried this code:

pub fn read_tag(path: String) -> Option<TagReadResponse> {
    let path = Path::new(&path);

    if !path.is_file() {
        return None;
    }

    let tagged_file = Probe::open(path)
        .expect("no probe")
        .read()
        .expect("no read");

    let tag = tagged_file.primary_tag().expect("no primary tag");

    ...
}

Summary

Trying to read the attached file gives this error:

PanicException(no read: Id3v2(ID3v2: BadFrameLength)

Expected behavior

I'm not sure this is actually a bug. This might just be a corrupted file and lofty is doing the right thing here but it seems like other software is able to read the tag and play the file. Maybe there's a way to skip a bad frame or something in this case?

ffprobe is able to read something:

ffprobe version 7.0.2 Copyright (c) 2007-2024 the FFmpeg developers
  built with Apple clang version 15.0.0 (clang-1500.3.9.4)
  configuration: --prefix=/opt/homebrew/Cellar/ffmpeg/7.0.2 --enable-shared --enable-pthreads --enable-version3 --cc=clang --host-cflags= --host-ldflags='-Wl,-ld_classic' --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libaribb24 --enable-libbluray --enable-libdav1d --enable-libharfbuzz --enable-libjxl --enable-libmp3lame --enable-libopus --enable-librav1e --enable-librist --enable-librubberband --enable-libsnappy --enable-libsrt --enable-libssh --enable-libsvtav1 --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxml2 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-libspeex --enable-libsoxr --enable-libzmq --enable-libzimg --disable-libjack --disable-indev=jack --enable-videotoolbox --enable-audiotoolbox --enable-neon
  libavutil      59.  8.100 / 59.  8.100
  libavcodec     61.  3.100 / 61.  3.100
  libavformat    61.  1.100 / 61.  1.100
  libavdevice    61.  1.100 / 61.  1.100
  libavfilter    10.  1.100 / 10.  1.100
  libswscale      8.  1.100 /  8.  1.100
  libswresample   5.  1.100 /  5.  1.100
  libpostproc    58.  1.100 / 58.  1.100
Input #0, mp3, from 'sample.mp3':
  Metadata:
    TDTG            : 2014-06-10T02:16:10
    encoder         : Lavf53.21.1
    artist          : Elvis Crespo
    title           : Suavemente
    genre           : Merengue;Latin;Salsa;Dance
    minor_version   : 0
    major_brand     : mp42
    compatible_brands: isommp42
    album           : Suavemente
    album-sort      : Suavemente
  Duration: 00:04:28.36, start: 0.000000, bitrate: 193 kb/s
  Stream #0:0: Audio: mp3 (mp3float), 44100 Hz, stereo, fltp, 192 kb/s
  Stream #0:1: Video: mjpeg (Baseline), yuvj444p(pc, bt470bg/unknown/unknown), 600x600 [SAR 72:72 DAR 1:1], 90k tbr, 90k tbn (attached pic)
      Metadata:
        comment         : Other

Assets

sample.mp3.zip

[Serial-ATA]

The file does have an invalid TDEN frame, but that's not actually an issue. The problem is, the frame fails to be parsed and the parser is told to skip it. Except it ends up trying to skip the entire frame size... even though we already read some of it.

The frame parsing is done with a restricted reader that cannot go outside the bounds of a frame. The frame skipping is not restricted, so it's actually able to read outside the bounds of the frame.

So, what's happening is:

1. Try to parse the TDEN frame (length of 1)
2. Create a reader restricted to the length of TDEN
3. Read the frame content
4. The frame is invalid, report it as such
5. Discard the restricted reader and tell the parser to skip the frame
6. End up skipping 1 byte ahead, even though we already read it
7. We've now eaten into the next frame and the parser freaks out

Don't know how this wasn't caught by tests, I definitely have to add some.

[milesegan]

Thanks very much for another super fast response! I'm glad I didn't try to debug this one myself.

[milesegan]

The fix works and I can now scan that file. Thanks!

One odd thing - the audio_bitrate is coming back as 32 although I think it should be 192 as reported by ffprobe. Could that be related to this issue or is it likely to be unrelated?

[Serial-ATA]

That'd be unrelated. The stream is likely VBR, which isn't supported yet (they just get treated like CBR streams for now).

lofty-rs/lofty/src/mpeg/properties.rs

Lines 216 to 221 in 06618cf

	if !is_cbr {
	log::debug!("MPEG: VBR detected");
	// TODO: Actually handle VBR streams, this still assumes CBR
	properties.audio_bitrate = first_frame_header.bitrate;
	}