Read the first three bytes of mini box directly

[wantehchang]

The first three bytes of the mini box can be read directly without checking any condition.

[wantehchang]

The easy parsing ends with the width_minus1 field. There are two ways to extend the easy parsing to the height_minus1 field:

1. Waste one bit and split small_dimensions_flag into small_width_flag and small_height_flag, or
2. Move the few_codec_config_bytes_flag field before the height_minus1 field.

[wantehchang]

@podborski Dimitri: Please see this comment. I just thought of a third way to extend byte alignment to the height_minus1 field:

• Change small_dimensions_flag ? 7 : 15 to small_dimensions_flag ? 8 : 16 for width_minus1 and height_minus1. One justification for this is that the maximum AV1 frame width and height are 2^16. This will require moving one of the boolean flags in the first two bytes later, so that small_dimensions_flag can be moved to the end of the second byte.

[podborski]

Thanks for catching this @wantehchang the intention was indeed to have it byte aligned including the dimension signaling. We should definitely bring an input contribution on this issue to the next meeting with the possible solutions and our preference for one of the solutions. To me it feels like having a separate flag for width and height is the better choice as there could be cases where only one of them can use 7 bits instead of 15. I'm not sure if extending the range for dimensions could be well accepted as the main use-case for slimHEIF was to target small files. Also slimHEIF is codec agnostic, not sure if we can motivate using AV1 limitations.

[wantehchang]

@podborski Thanks for the confirmation. Note that small_dimensions_flag is used to control the size of the gainmap_width_minus1 and gainmap_height_minus1 fields later in the mini box., but byte alignment is already lost at that point. So I don't think we need to worry about the byte alignment of those two fields.

[y-guyon]

I do not understand the purpose of this change. Offsets and masks do not look like easier parsing to me.
If the macro calls are too long, we could add another macro that bundles the error code, or make avifROStreamRead*() return an avifResult.

[wantehchang]

What I meant by "easy parsing" is really "simple parsing". Whether the new code is easy to understand depends on familiarity with bitwise operations.

The purpose of this change is to take advantage of Dimitri's change to the SlimHEIF proposal "Flags after version to be able to parse a fixed number of bytes right away". When I compared your pull request #2376 with the SlimHEIF proposal, I noticed that the first few fields in the mini box are byte-aligned and that reminded me of Dimitri's change.

[maryla-uc]

I also fail to see what this improves. The code is definitely harder to read. What's the advantage of "reading 3 bytes at once" compared to the current version? Surely from a performance point of view it makes so little difference it doesn't matter.
This design might make parsing "easier" for simple libraries that don't have the convenient bit reading functions like avifROStreamReadBits. But since we have all that already I don't think making the code harder to understand achieves anything.

[wantehchang]

Maryla, Yannis: Thank you for your comments. I have converted this pull request to a draft and will use it to discuss boolean field packing with authors of the SlimHEIF proposal.

As this pull request shows, the byte alignment ends with the width_minus1 field. The proponents of boolean field packing may have intended byte alignment to extend to the height_minus1 field. I am not sure if they know byte alignment ends before that.

[wantehchang]

I figured out how to make the code easy to understand: #2408