AOMediaCodec / av1-avif

AV1 Image File Format Specification - ISO-BMFF/HEIF derivative
https://aomediacodec.github.io/av1-avif/
BSD 2-Clause "Simplified" License
463 stars 40 forks source link

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

Closed baumanj closed 2 years ago

baumanj commented 2 years ago

Now that https://github.com/MPEGGroup/FileFormat/issues/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 https://github.com/gpac/ComplianceWarden/issues/33

leo-barnes commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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