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.

Sign up for GitHub

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

Jump to bottom

Allow Hdlr to be not the first in the Meta box #95

Merged
merged 2 commits into from
Jul 29, 2023
Merged

Allow Hdlr to be not the first in the Meta box #95

merged 2 commits into from
Jul 29, 2023

Conversation

DCNick3
Copy link
Contributor

@DCNick3 DCNick3 commented Feb 14, 2023

The spec seems to allow the hdlr box to be located at any place in the meta box. This commit addresses that.

@DCNick3 DCNick3 force-pushed the hdlr-any-location branch 2 times, most recently from 571d2db to 3a900c6 Compare February 14, 2023 21:36
@DCNick3
Copy link
Contributor Author

DCNick3 commented Feb 14, 2023

cc @jessa0 (as you are the one who wrote the code originally in #91): is assuming that contents of the Meta box are all boxes a sane approach? I don't think this can be parsed in any other way..

@jessa0
Copy link
Contributor

jessa0 commented Feb 14, 2023

The spec seems to allow the hdlr box to be located at any place in the meta box.

I'm not sure how you're interpreting the spec in that way? According to the syntax description in ISO 14496-12-2012 section 8.11.1.2, the HandlerBox comes first in the MetaBox, followed by a bunch of optional typed boxes, and a Box[] at the end. I don't have access to a spec of the syntax description language, but everywhere else in the spec, ordering in the class determines the order of data in the serialized file..

@DCNick3
Copy link
Contributor Author

DCNick3 commented Feb 14, 2023 •
edited
Loading

Oh, I've actually look at the verbal description instead of the syntactic one. The verbal one said "The 'meta' box is required to contain a ‘hdlr’ box indicating the structure or format of the ‘meta’ box contents.", so I figured "contain" is "contains anywhere"

The syntactic one does give an explicit order though...

So... Apparently this is not according to standard, but I do have a file, which seems to be produced by TMPGEnc Video Mastering Works 7 Version 7.0.15.17 (as indicated by the metadata) which does have the ilst box before the HandlerBox

I think it's still a good idea to support it, as the spec still guarantees that the contents of the MetaBox are all boxes

@jessa0
Copy link
Contributor

jessa0 commented Feb 15, 2023

So... Apparently this is not according to standard, but I do have a file, which seems to be produced by TMPGEnc Video Mastering Works 7 Version 7.0.15.17 (as indicated by the metadata) which does have the ilst box before the HandlerBox

Oh yikes, so I see we're out of spec territory then. Since you do have a sample from a real encoder with this behavior, I do agree with "being liberal with what you accept" here.

To actually answer your original question, yes, all the data should at least be Boxes, so your PR seems like an improvement in that respect.

@mrjackwills mrjackwills mentioned this pull request Apr 25, 2023
Closed
DCNick3 added a commit to DCNick3/shin that referenced this pull request Jun 17, 2023
@DCNick3
Chore: update git deps
a0b420f 
- enum-map to a newer version of the GAT PR (KamilaBorowska/enum-map#35)
- mp4 to the rebased Hdlr PR (alfg/mp4-rust#95)
@w-flo w-flo mentioned this pull request Jun 29, 2023
Merged
@DCNick3
Allow Hdlr to be not the first in the Meta box
d735464 
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
@DCNick3
Copy link
Contributor Author

DCNick3 commented Jul 28, 2023

Rebased and squashed, CI fails again due to an unrelated lint =(

@alfg alfg merged commit c104047 into alfg:master Jul 29, 2023
1 check passed
@alfg
Copy link
Owner

alfg commented Jul 29, 2023

Thanks @DCNick3 and @jessa0.

pando-fredrik pushed a commit to pando-fredrik/mp4-rust that referenced this pull request May 13, 2024
@DCNick3 @alfg @Na1w
Allow Hdlr to be not the first in the Meta box (alfg#95)
46d9ec9 
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 <[email protected]>
pando-fredrik pushed a commit to pando-fredrik/mp4-rust that referenced this pull request May 13, 2024
@DCNick3 @alfg @Na1w
Allow Hdlr to be not the first in the Meta box (alfg#95)
7585bab 
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 <[email protected]>
pando-fredrik added a commit to pando-fredrik/mp4-rust that referenced this pull request May 13, 2024
@pando-fredrik @kixelated
@LinusU @rolleifx @DCNick3 @alfg @w-flo @jensenn @emkman99 @andreytkachenko
Pr 103 multiple trex (#3)
9ebaabc 
* 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 <[email protected]>

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

Fixes alfg#102

Co-authored-by: Alfred Gutierrez <[email protected]>

* 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 <[email protected]>

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

Co-authored-by: Alfred Gutierrez <[email protected]>

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

Co-authored-by: Alfred Gutierrez <[email protected]>

* Fix writing SLConfigDescriptor (alfg#107)

Co-authored-by: Alfred Gutierrez <[email protected]>

* 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 <[email protected]>

* hev1 box parser (alfg#101)

Co-authored-by: Alfred Gutierrez <[email protected]>

---------

Co-authored-by: Luke Curley <[email protected]>
Co-authored-by: Linus Unnebäck <[email protected]>
Co-authored-by: rolleifx <[email protected]>
Co-authored-by: ⭐️NINIKA⭐️ <[email protected]>
Co-authored-by: Alfred Gutierrez <[email protected]>
Co-authored-by: w-flo <[email protected]>
Co-authored-by: jensenn <[email protected]>
Co-authored-by: emkman99 <[email protected]>
Co-authored-by: Andrey Tkachenko <[email protected]>
jprochazk pushed a commit to jprochazk/mp4 that referenced this pull request Sep 18, 2024
@DCNick3 @alfg
Allow Hdlr to be not the first in the Meta box (alfg#95)
6c92bf7 
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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alfg alfg alfg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@DCNick3 @jessa0 @alfg