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

possible bug in m4a parsing #482

Closed
milesegan opened this issue Nov 4, 2024 · 9 comments · Fixed by #486
Closed

possible bug in m4a parsing #482

milesegan opened this issue Nov 4, 2024 · 9 comments · Fixed by #486
Labels
bug Something isn't working
Milestone

Comments

@milesegan
Copy link

milesegan commented Nov 4, 2024

Reproducer

I tried this code:

fn main() {
    // the error seems to occur with strict or relaxed parsing mode
    let parsing_options = ParseOptions::new().parsing_mode(ParsingMode::Relaxed);
    let path = Path::new("sample.m4a");
    let file = Probe::open(&path)
        .expect("can't probe")
        .options(parsing_options)
        .read()
        .expect("can't read");
    let tag = file.primary_tag().expect("no tag");
    let properties = file.properties();
    println!("{:?}", tag.artist());
    println!("{:?}", tag.year());
}

Summary

When trying to read the file I get this panic:

no read: BadAtom("\"covr\" atom has an unknown type")
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::result::unwrap_failed
   3: core::result::Result<T,E>::expect
   4: rust_lib_spinner::api::tagging::read_tag

Expected behavior

I don't doubt that there is a bad atom here. Could it be skipped in relaxed mode?

ffprobe output:

[mov,mp4,m4a,3gp,3g2,mj2 @ 0x140e06d30] Unknown cover type: 0x15.
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 'sample.m4a':
  Metadata:
    major_brand     : isom
    minor_version   : 512
    compatible_brands: isomiso2mp41
    album_artist    : BADBADNOTGOOD
    disc            : 1/24576
    track           : 1/24576
    artist          : BADBADNOTGOOD
    album           : III
    date            : 2014
    title           : Triangle
    encoder         : Lavf55.14.102
    genre           : Hip Hop
  Duration: 00:03:47.00, start: 0.000000, bitrate: 514 kb/s
  Stream #0:0[0x1](und): Audio: aac (LC) (mp4a / 0x6134706D), 44100 Hz, stereo, fltp, 511 kb/s (default)
      Metadata:
        handler_name    : SoundHandler
        vendor_id       : [0][0][0][0]

Assets

sample.zip

@milesegan milesegan added the bug Something isn't working label Nov 4, 2024
@Serial-ATA
Copy link
Owner

I don't doubt that there is a bad atom here. Could it be skipped in relaxed mode?

Definitely. Just another case of an error that existed before ParsingMode. :)

I am curious, do you know the source of the file? Or if any player can tell what the cover is supposed to be? I wonder how it got that data type.

@Serial-ATA Serial-ATA added this to the 0.22.0 milestone Nov 4, 2024
@milesegan
Copy link
Author

@Serial-ATA Good question I'll try to find the source. Apple Music does seem to be able to display the artwork when I load it there.

@milesegan
Copy link
Author

Apparently this file originated in Apple Music so I suppose it's not surprising that it shows up there.

@Serial-ATA
Copy link
Owner

The cover is just a PNG, so I wonder if Apple just ignores data types or introduced a new one for some reason. I don't have Apple Music, could you see if it load this file? test.zip

I just took one of the benchmark assets and set a wrong data type, the cover is still a PNG.

@milesegan
Copy link
Author

Thanks for the sample. The artwork for the file in your test.zip shows up in both the finder and in Apple Music. Interestingly, the artwork for the file I attached shows up only in Apple Music and not in the finder.

Apple Music supports both static images and in some cases animated images for artwork. Maybe this non-standard COVR atom has something to do with that?

@Serial-ATA
Copy link
Owner

The artwork for the file in your test.zip shows up in both the finder and in Apple Music.

I guess that means they ignore data types and try to read images anyway

Apple Music supports both static images and in some cases animated images for artwork. Maybe this non-standard COVR atom has something to do with that?

I was thinking it might have to do with that, but this is just a normal static PNG so there shouldn't be a reason to use anything but the data type for PNG.

I haven't found anyone else talking about this so far, but I imagine if Apple Music is producing these files there's probably a reason?

@milesegan
Copy link
Author

I suppose it could also just be another case of Apple being Apple.

What do you think is the best way to handle these? Check and see if the data looks like an image and parse it or just skip it?

@Serial-ATA
Copy link
Owner

I suppose it could also just be another case of Apple being Apple.

That's true. There are multiple hacks in here to try and support old iTunes bugs.

What do you think is the best way to handle these?

For now I'll just skip it, most others do that anyway. Do you have this issue with other files from Apple Music? If so, I imagine other people will report similar things.

@milesegan
Copy link
Author

So far this is the only case I've had reported of this issue.

Thanks yet again for such a quick and helpful response!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants