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

Invalid test files: False negative: HandlerBox (hdlr) name field shall not contain NUL bytes prior to terminator #174

Closed
baumanj opened this issue Nov 20, 2021 · 4 comments · Fixed by #175
Labels

Comments

@baumanj
Copy link
Collaborator

baumanj commented Nov 20, 2021

Now that MPEGGroup/FileFormat#35 has clarified the specification, I believe the files in https://github.com/AOMediaCodec/av1-avif/blob/master/testFiles/Apple/multilayer_examples all have technically invalid hdlr boxes:

00 00 00 22 // Box size (34 bytes)
68 64 6c 72 // boxtype ("hdlr")
00 00 00 00 // version (1 byte), flags (3 bytes)
00 00 00 00 // pre_defined = 0
70 69 63 74 // handler_type ("pict")
00 00 00 00 // reserved[0] = 0
00 00 00 00 // reserved[1] = 0
00 00 00 00 // reserved[2] = 0
00 00       // name ("\0\0")

@leo-barnes: what writer generated this container?

See also gpac/ComplianceWarden#33

@leo-barnes
Copy link
Collaborator

It's a small script we use to generate test content. I'll need to check if this matches how we write our HEIF files.

Regarding the compliance warden issue, I added the following comment there:

I'm not sure I agree that this is non-spec compliant though (although it might be good to output a warning). Since name is a NULL terminated string, it will end at the first \0. I don't think there is anything in the spec disallowing extra data to be in a box. If there is, then I agree that this is non-compliant. But if there isn't it's just extra unused cruft in the box. (Still not ideal of course.)

If name was something like lib\0avif\0, I would expect the avif\0 to simply be ignored.

@cconcolato
Do you happen to know what the spec says regarding extra data in boxes?

@baumanj
Copy link
Collaborator Author

baumanj commented Nov 22, 2021

If the HEIC I just captured on my iOS 14.7.1 device is any indication, it looks like this extra byte is always present:

$ hexdump -C -n 82 IMG_9394.HEIC 
00000000  00 00 00 24 66 74 79 70  68 65 69 63 00 00 00 00  |...$ftypheic....|
00000010  6d 69 66 31 4d 69 50 72  6d 69 61 66 4d 69 48 42  |mif1MiPrmiafMiHB|
00000020  68 65 69 63 00 00 10 da  6d 65 74 61 00 00 00 00  |heic....meta....|
00000030  00 00 00 22 68 64 6c 72  00 00 00 00 00 00 00 00  |..."hdlr........|
00000040  70 69 63 74 00 00 00 00  00 00 00 00 00 00 00 00  |pict............|
00000050  00 00                                             |..|

The giveaway is that the length of the hdlr box is 34 (hex 00 00 00 22), when a minimal hdlr box would have length 33.

@leo-barnes
Copy link
Collaborator

Yep, I came to the same conclusion. It seems like the same is true for at least some videos as well. I'm starting a conversation with the video folks to see if we can fix it.

@leo-barnes
Copy link
Collaborator

Good catch by the way! This has most likely been the case for many many years.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants