search

AOMediaCodec / iamf

Immersive Audio Model and Formats
https://aomediacodec.github.io/iamf/
79 stars 15 forks source link

Suggestion to consider changing leb128 in IACB atom to a fixed 4-byte value #818

Closed shawnisingh closed 3 months ago

shawnisingh commented 3 months ago

It's been suggested to me that variable size information in mp4 boxes is unconventional and more difficult for some parsers to deal with.

I know that v1 spec is long since frozen and published, but I still want to ask if there's any chance it can change from leb128 to fixed 4 bytes. Or, if there is any other option to remediate this?

tdaede commented 3 months ago

Variable length data is pretty common for most codec configuration boxes at this point - av1 allows a list of OBUs, for example.

lotharkript commented 3 months ago

The MP4 box already contains a size. The size can be uint32 or uint64.

Why the need to have another file in the iacb box, called leb128() configOBUs_size;

when we could get the size from the box itself.

Also, most other codecs do not seems to have a size for their decoder config (for example, avcC, hvcC). And if some box need some size, most of the time, the field with be a uint32 to store it. What we are saving by using a leb128 field to store the size? Maybe 3 bytes over the whole file?

The question is why do we need a variable size field to store a size? Can we not either not have the field and use the box size? Could we not have a uint32 field to store the size?

jwcullen commented 3 months ago

A lot of the context of this field was discussed in #792.

Having the configOBUs_size field may allow extensions if needed in the future. So those extensions would result in a difference between the iacb box size and the configOBUs_size with the extension bytes after configOBUs.

lotharkript commented 3 months ago

I like the comment in the #792

// leb128() should save a few bytes compared to unsigned int (32)

Now, for Mp4 demuxer, we have to add support for leb128 just to save 2 or 3 bytes and make the muxing a little bit more complex as we will need to know how many byte the configOBUs_size will be when written in leb128 before setting the size of the box, versus just using an int (32) which make the easier to read and write and validate the box.

Otherwise, to validate the box, we will need to read the leb128 field, to know how many byte the size was store, to compare the total box size.

tdaede commented 3 months ago

For writing, you can always pad the LEB128.

I mean, maybe it's not worth saving the size for a future version, but this doesn't really seem worth making a rev of the config box for.

sunghee-hwang commented 3 months ago

I am going to close this issue this week. If you have a concern, please feel free to leave a comment.