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.

Sign up for GitHub

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

Write the MA1B brand correctly #2515

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Write the MA1B brand correctly #2515

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3bddabc
Write the MA1B brand correctly
wantehchang Dec 2, 2024
5fd0303
Run clang-format
wantehchang Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions src/write.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3134,6 +3134,7 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)
// -----------------------------------------------------------------------
// Harvest configuration properties from sequence headers

avifBool isMA1B = AVIF_TRUE;
for (uint32_t itemIndex = 0; itemIndex < encoder->data->items.count; ++itemIndex) {
avifEncoderItem * item = &encoder->data->items.item[itemIndex];
if (item->encodeOutput->samples.count > 0) {
Expand All @@ -3142,6 +3143,11 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)
AVIF_CHECKERR(avifSequenceHeaderParse(&sequenceHeader, (const avifROData *)&firstSample->data, codecType),
avifGetErrorForItemCategory(item->itemCategory));
item->av1C = sequenceHeader.av1C;
// The MA1B brand: The AV1 profile shall be the Main Profile and the level shall be 5.1
// or lower.
if (item->av1C.seqProfile != 0 || item->av1C.seqLevelIdx0 > 13) {
isMA1B = AVIF_FALSE;
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is my understanding:

  • A track exists in avifEncoder only 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.

}
}

Expand Down Expand Up @@ -3226,24 +3232,25 @@ avifResult avifEncoderFinish(avifEncoder * encoder, avifRWData * output)

avifBoxMarker ftyp;
AVIF_CHECKRES(avifRWStreamWriteBox(&s, "ftyp", AVIF_BOX_SIZE_TBD, &ftyp));
AVIF_CHECKRES(avifRWStreamWriteChars(&s, majorBrand, 4)); // unsigned int(32) major_brand;
AVIF_CHECKRES(avifRWStreamWriteU32(&s, minorVersion)); // unsigned int(32) minor_version;
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avif", 4)); // unsigned int(32) compatible_brands[];
if (useAvioBrand) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avio", 4)); // ... compatible_brands[]
} //
if (isSequence) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avis", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "msf1", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "iso8", 4)); // ... compatible_brands[]
} //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "mif1", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "miaf", 4)); // ... compatible_brands[]
if ((imageMetadata->depth == 8) || (imageMetadata->depth == 10)) { //
if (imageMetadata->yuvFormat == AVIF_PIXEL_FORMAT_YUV420) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "MA1B", 4)); // ... compatible_brands[]
} else if (imageMetadata->yuvFormat == AVIF_PIXEL_FORMAT_YUV444) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "MA1A", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, majorBrand, 4)); // unsigned int(32) major_brand;
AVIF_CHECKRES(avifRWStreamWriteU32(&s, minorVersion)); // unsigned int(32) minor_version;
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avif", 4)); // unsigned int(32) compatible_brands[];
if (useAvioBrand) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avio", 4)); // ... compatible_brands[]
} //
if (isSequence) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "avis", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "msf1", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "iso8", 4)); // ... compatible_brands[]
} //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "mif1", 4)); // ... compatible_brands[]
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "miaf", 4)); // ... compatible_brands[]
if (isMA1B) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "MA1B", 4)); // ... compatible_brands[]
} //
if ((imageMetadata->depth == 8) || (imageMetadata->depth == 10)) { //
if (imageMetadata->yuvFormat == AVIF_PIXEL_FORMAT_YUV444) { //
AVIF_CHECKRES(avifRWStreamWriteChars(&s, "MA1A", 4)); // ... compatible_brands[]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
}
#if defined(AVIF_ENABLE_EXPERIMENTAL_GAIN_MAP)
Expand Down
Loading