Write the MA1B brand correctly#2515
Conversation
A partial fix for AOMediaCodec#2514.
wantehchang
force-pushed
the
write-MA1B-brand-correctly
branch
from
984b8ae to
3bddabc
Compare
wantehchang
left a comment
wantehchang
left a comment
There was a problem hiding this comment.
Yannis: To help code review, I split the pull request into two commits. Commit 1 is the actual change. Commit 2 is clang-format only.
| // or lower. | ||
| if (item->av1C.seqProfile != 0 || item->av1C.seqLevelIdx0 > 13) { | ||
| isMA1B = AVIF_FALSE; | ||
| } |
There was a problem hiding this comment.
Yannis: I am not that familiar with this code. It is important that this for loop (which starts at line 3138) is executed for both image items and image sequences, and that we enter the if (item->encodeOutput->samples.count > 0) block (which starts at line 3140) at least once. Otherwise we will incorrectly report that isMA1B is true (the initial value). Please check this. Thanks!
There was a problem hiding this comment.
Here is my understanding:
- A track exists in
avifEncoderonly if the corresponding item is present too, likely with the first frame of the sequence being used as the image item data. - There is no change of profile/level within a single sequence.
Assuming this is correct, the for loop and the MA1B check look good.
| } // | ||
| if ((imageMetadata->depth == 8) || (imageMetadata->depth == 10)) { // | ||
| if (imageMetadata->yuvFormat == AVIF_PIXEL_FORMAT_YUV444) { // | ||
| AVIF_CHECKRES(avifRWStreamWriteChars(&s, "MA1A", 4)); // ... compatible_brands[] |
There was a problem hiding this comment.
Are the constraints listed in https://aomediacodec.github.io/av1-avif/v1.1.0.html#advanced-profile always true here?
Note that they differ for items and sequences.
| // or lower. | ||
| if (item->av1C.seqProfile != 0 || item->av1C.seqLevelIdx0 > 13) { | ||
| isMA1B = AVIF_FALSE; | ||
| } |
There was a problem hiding this comment.
Here is my understanding:
- A track exists in
avifEncoderonly if the corresponding item is present too, likely with the first frame of the sequence being used as the image item data. - There is no change of profile/level within a single sequence.
Assuming this is correct, the for loop and the MA1B check look good.
2 participants
A partial fix for #2514.