search

saecki / mp4ameta

A library for reading and writing iTunes style MPEG-4 audio metadata
Apache License 2.0
35 stars 5 forks source link

Two types of errors on real-world m4a file metadata #2

Closed rayrrr closed 4 years ago

rayrrr commented 4 years ago

Hello again, after the previous fix for #1 I am now encountering two distinct kinds of errors with all the M4A files in my collection, either

Parsing: Typed data head to short

or

UnknownDataType(21): Unknown datatype code

I am providing two test files, one that produces each of the above errors, at https://github.com/rayrrr/rust-mp4ameta/tree/master/test_files so please advise. Thank you again for your efforts.

rayrrr commented 4 years ago

I will attempt a Pull Request myself here but happy to let you fix it as well. I see the error messages were emitted by the data.rs file in this crate and am digging in. First question: should https://github.com/Saecki/rust-mp4ameta/blob/master/src/data.rs#L35 read Unparsed(u32) instead of i32 as per the comment directly above it? Or is it the comment that is incorrect?

saecki commented 4 years ago

Sorry, I am only seeing this now. Parsing: Typed data head to short

The 21 in UnknownDataType(21): Unknown datatype code error stands for a BE Signed Integer according to this table, which should probably be implemented.

The the comment on the Unparsed(i32) should be i32 since -1 represents typed data which datatype is yet to be read in the first 4 bytes of a data atoms content.

Thanks for reporting, I'm at it.

saecki commented 4 years ago

Oh I took a look at your changes and saw you already implemented most of what I said so this probably sounded a bit ignorant. I will try to merge your current changes and fix it from there.

rayrrr commented 4 years ago

Caveat emptor...I am very new to Rust and my fork is not compiling in its current state. I am sure I correctly transcribed the additional atom constants but that's about all I can guarantee. Thank you for working on this.

saecki commented 4 years ago

I created a bugfix branch which at least compiles now.

rayrrr commented 4 years ago

Definite progress on that bugfix branch. The ones that used to panic with UnknownDataType(21): Unknown datatype code are being handled correctly. And now the other one is returning a more detailed panic error:

Parsing: Error reading moov: Error reading udta: Error reading meta: Error reading ilst: Error reading ©wrt: Error reading data: Typed data head to short', src/main.rs:5:17

Wishing you luck and looking forward to the next version bump. Thank you so much!

saecki commented 4 years ago

I think I found the bug, I was incorrectly checking if typed data length was > 8 bytes instead of >= 8 bytes. So empty data atoms would result in the Parsing: Typed data head to short error. If this works for you I will merge it into master and bump the version number.

saecki commented 4 years ago

Version 0.2.2 containing these fixes is now published.

rayrrr commented 4 years ago

It is now working correctly with all the files that were exhibiting these errors. Thanks so much @Saecki !