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

Error: BoxNotFound(hdlr): Not found hdlr #102

Closed
ajosecueto opened this issue Apr 19, 2023 · 18 comments · Fixed by #111
Closed

Error: BoxNotFound(hdlr): Not found hdlr #102

ajosecueto opened this issue Apr 19, 2023 · 18 comments · Fixed by #111

Comments

@ajosecueto
Copy link

I'm getting this error using videos recoded from phone camera or screen recording -> BoxNotFound(hdlr)
This error doesn't appear when i'm using videos downloaded from social networks or other platfroms.

Ocurs using mp4 = "0.13.0" but read_header works in mp4 = "0.12.0" with all kind of mp4 videos.

let f = std::fs::File::open(temp_file.path()).unwrap();
let size = f.metadata()?.len();
let reader = BufReader::new(f);

let mp4 = mp4::Mp4Reader::read_header(reader, size)?; // -> the error ocurs here 
@mrjackwills
Copy link

mrjackwills commented Apr 25, 2023

I am also running into this issue, working fine in 0.12, but 0.13 introduces some issue. I think I have narrowed it down to this code block, from meta.rs, but other than that I am at a loss.

When this is executed against my MP4 file, the hdlr_header.name is empty, the BoxHeader::read function is reading typ as 0, which it then can't convert into a BoxType.

 let hdlr_header = BoxHeader::read(reader)?;
        if hdlr_header.name != BoxType::HdlrBox {
            return Err(Error::BoxNotFound(BoxType::HdlrBox));
        }

I have also tried the Allow Hdlr to be not the first in the Meta box PR, and am still getting the same error.

EDIT: I have uploaded a "bad" 1 second mp4, https://www.mrjackwills.com/bad_mp4.mp4, it was shot with a Google Pixel 4a 5G phone.

@DCNick3
Copy link
Contributor

DCNick3 commented Jun 17, 2023

I can't download the mp4 file from this URL, it just serves me an HTML with a webapp redirecting to the main page

@mrjackwills
Copy link

I can't download the mp4 file from this URL, it just serves me an HTML with a webapp redirecting to the main page

Argh, sorry about that, I deleted the file by accident, and I need to fix something on the webapp to make it work correctly (PWA cache busting).
As the GitHub file size limit is 10mb, I have a super short 3mb file here, also the same file in a zip, in case GitHub does anything funky with compression etc

small_bad_mp4.mp4

small_bad_mp4.zip

@DCNick3
Copy link
Contributor

DCNick3 commented Jun 24, 2023

Briefly looking at the file in MP4Box.js... It appears the file has an empty meta box in the moov box.

image

This code tries to read it as any other meta box, but as it's empty and doesn't have the hdrl box, it fails.

Now I guess is the time to look at the spec and try to figure out what is the correct behavior here...

@DCNick3
Copy link
Contributor

DCNick3 commented Jun 24, 2023

ISO/IEC 14496-12:2015 in section 8.11.1 clearly states, seemingly without any exceptions:

The 'meta' box is required to contain a ‘hdlr’ box indicating the structure or format of the ‘meta’ box contents.

It appears to be similar problem to #95 when implementers are interpreting the spec very liberally... In which case it's better to look at actual implementations like ffmpeg

@mrjackwills
Copy link

ISO/IEC 14496-12:2015 in section 8.11.1 clearly states, seemingly without any exceptions:

The 'meta' box is required to contain a ‘hdlr’ box indicating the structure or format of the ‘meta’ box contents.

It appears to be similar problem to #95 when implementers are interpreting the spec very liberally... In which case it's better to look at actual implementations like ffmpeg

Can I ask a slightly unrelated question. Where can one read the ISO spec, without having to pay over 200 Swiss Francs from the ISO official website?

@DCNick3
Copy link
Contributor

DCNick3 commented Jun 26, 2023

Can I ask a slightly unrelated question. Where can one read the ISO spec, without having to pay over 200 Swiss Francs from the ISO official website?

I don't think you are supposed to be able to. But you can find the PDFs in google if you look hard enough

@mrjackwills
Copy link

Can I ask a slightly unrelated question. Where can one read the ISO spec, without having to pay over 200 Swiss Francs from the ISO official website?

I don't think you are supposed to be able to. But you can find the PDFs in google if you look hard enough

Haha, I thought that might be the case

@w-flo
Copy link
Contributor

w-flo commented Jun 29, 2023

My phone (Android-based) produces mp4 files like this, too.

It turns out they, like the mp4 file attached in a comment above, are not actually missing the hdlr box. Instead, something is odd with the meta box. Look at the file in a hex editor. As I understand the spec, the meta box should extend FullBox. That means the 4 bytes meta should be followed by 1 byte for the version number and 3 bytes for flags (all 0). However, in this case, the meta box seems to be extending Box only, which means there is no version number and no flags, and the next 4 bytes in the file are already the size field for the hdlr box (33).

So the call to read_box_header_ext() would probably need to be skipped in this case.

It's interesting that this seems to work in other software. I've looked at the ffmpeg code, it looks like for reading a meta box, they just scan through the file from the start of the meta box until they find the "hdlr" bytes, then jump back 8 bytes to reset the read position to the start of the hdlr box and parse it. If no "hdlr" bytes are found, the function returns without error.

Maybe some mp4 variants simply use Box instead of FullBox for the meta box? My phone says the ftyp is mp42, and the "small_bad_mp4.mp4" attached above says isom.

Edit: I suspect the quicktime format, which is of course similar to mp4, uses this meta format without the extended header. I do wonder why these files claim to be mp42 / isom, but still use the old quicktime format? Anyway, I guess the best way forward is to just handle this. Gstreamer seems to have some code that basically does:

  • if the next 4 bytes are 0x00000000, then this is the ISO extended header.
  • If the "next-next" 4 bytes are "hdlr", then this file still uses the old quicktime format for some reason and the extended ISO header for "FullBox" was skipped.
  • Everything else is weird and triggers a warning

w-flo added a commit to w-flo/mp4-rust that referenced this issue Jun 29, 2023
@w-flo
Copy link
Contributor

w-flo commented Jun 29, 2023

@ajosecueto @mrjackwills It would be interesting to know if #111 fixes this issue for you. I've tested the small_bad_mp4.mp4 from this issue thread, and that one appears to be fixed now.

w-flo added a commit to w-flo/mp4-rust that referenced this issue Jun 29, 2023
@ajosecueto
Copy link
Author

@w-flo it works, i tested with some videos and works. No more Not found hdlr.

mp4 = { git = "https://github.com/alfg/mp4-rust.git", rev = "55ee392b73801d5aff41dbde36455917e0ac2d19" }

@mrjackwills
Copy link

@ajosecueto @mrjackwills It would be interesting to know if #111 fixes this issue for you. I've tested the small_bad_mp4.mp4 from this issue thread, and that one appears to be fixed now.

Sorry, I've not had proper access to my computer or the internet for a while, but I can confirm that it appears to now work when using the following dependency;

mp4 = { git = "https://github.com/alfg/mp4-rust.git", rev = "55ee392b73801d5aff41dbde36455917e0ac2d19" }

@w-flo
Copy link
Contributor

w-flo commented Jul 27, 2023

Great! Thanks to both of you for testing. Hopefully this will give the PR a good chance of being merged.

@alfg
Copy link
Owner

alfg commented Jul 28, 2023

Thanks, all. I've been super busy, but I'll try to make time to review and merge soon. Sorry for the wait!

@mrjackwills
Copy link

@alfg @w-flo I only tested it against "bad" mp4's, haven't looked into the source of the pr fully, but assume "good" mp4's will still work as before, can test again later

@alfg
Copy link
Owner

alfg commented Jul 29, 2023

@mrjackwills Thanks. I have a set of "good" mp4s that I use to validate that I will run through. I've been meaning to create a new set of integration tests for these as well.

@alfg alfg closed this as completed in #111 Jul 29, 2023
alfg added a commit that referenced this issue Jul 29, 2023
Fixes #102

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>
@alfg
Copy link
Owner

alfg commented Jul 29, 2023

#95 and #111 are merged. Feel free to try again using the latest in master and I will create a release.

mp4 = { git = "https://github.com/alfg/mp4-rust.git", rev = "d6c38642de48360693132fd07552a454016d71c3" }
or
mp4 = { git = "https://github.com/alfg/mp4-rust", branch = "master" }

@w-flo
Copy link
Contributor

w-flo commented Jul 30, 2023

This issue is fixed for me when using the master branch, thanks a lot @alfg!

pando-fredrik pushed a commit to pando-fredrik/mp4-rust that referenced this issue May 13, 2024
Fixes alfg#102

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>
pando-fredrik pushed a commit to pando-fredrik/mp4-rust that referenced this issue May 13, 2024
Fixes alfg#102

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>
pando-fredrik added a commit to pando-fredrik/mp4-rust that referenced this issue May 13, 2024
* Add support for multiple trex boxes.

While it's common to construct a fMP4 file for a single track, it's
totally valid to have multiple tracks. This means you also need multiple
to support multiple trex boxes.

Untested.

* cargo fmt

* Add a missing write_box call for mvex.

* Ran clippy

* Fix more errors when serializing a mvex box.

udta is also broken but that can be fixed in another PR.

* Derive Default trait for DataType (alfg#100)

* fix clippy (rustc 1.71.0) (alfg#115)

* Allow Hdlr to be not the first in the Meta box (alfg#95)

While the spec says that the hdlr box should be the first one, not all
implementations follow that. Actually look over all boxes in Meta to
find Hdlr.

Also add a test for such weirdly-formatted box

Change the way unknown MetaBox is stored: store a list of boxes instead
of raw bytes

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Try to skip extended header in MetaBox. (alfg#111)

Fixes alfg#102

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Release 0.14.0 (alfg#117)

* Update Cargo.toml

* Update README.md

* Fix getting samples from movie fragments (alfg#106)

* Fix getting samples from movie fragments

* Add a function to the reader to read in fragments from a different reader

---------

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Export all boxes to allow more flexible use for writing (alfg#108)

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Fix some minor issues writing traf box (alfg#109)

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Fix writing SLConfigDescriptor (alfg#107)

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* Extract esds box from wave box (alfg#96)

* Extract esds from wave box

* Allow empty, multi-byte, and arbitrary NUL terminated strings

* Skip unsupported avc1 sub-boxes

* Fixed non-integer framerates

* Fixed bitrate calculation

* Fixed format issue

* Public read sample offset

* Fix lint warning.

---------

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

* hev1 box parser (alfg#101)

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>

---------

Co-authored-by: Luke Curley <kixelated@gmail.com>
Co-authored-by: Linus Unnebäck <linus@folkdatorn.se>
Co-authored-by: rolleifx <110799844+rolleifx@users.noreply.github.com>
Co-authored-by: ⭐️NINIKA⭐️ <DCNick3@users.noreply.github.com>
Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>
Co-authored-by: w-flo <w-flo@users.noreply.github.com>
Co-authored-by: jensenn <neil.jensen@pluto.tv>
Co-authored-by: emkman99 <emkman99@users.noreply.github.com>
Co-authored-by: Andrey Tkachenko <andrey@aidev.ru>
jprochazk pushed a commit to jprochazk/mp4 that referenced this issue Sep 18, 2024
Fixes alfg#102

Co-authored-by: Alfred Gutierrez <alfg@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants