Keep every target of a track 'auxl' or 'prem' reference (#3335)
avifTrackReferenceBox() lost track references in two ways. It read one
track_ID and skipped the rest of the box, and it assigned the result
unconditionally, so a second 'auxl' box overwrote what the first one had
recorded. avifParseTrackBox() keeps a seen flag for 'tkhd' and 'edts' but
not for 'tref', so several 'tref' boxes in one 'trak' did the same.
Section 8.3.3.2 of ISO/IEC 14496-12 gives TrackReferenceTypeBox an array
of track_IDs with no count field, so the number of entries follows from
the box size.
auxForID is compared against colorTrack->id when the alpha track is looked
up. If the color track was not the target that survived, the lookup found
nothing, alphaTrack stayed null and alphaPresent was false, with no
diagnostic and no error code. premByID has the same shape, with
alphaPremultiplied as its consumer.
Give avifTrack the reference array that avifDecoderItem already has, read
track_IDs until the box runs out, and ask the same helpers. Those helpers
are renamed and now take the array instead of the item so tracks can use
them unchanged, which is why the item call sites move.
A track_ID of 0 is skipped rather than recorded: auxForID was an in-band
sentinel where 0 meant "no reference", and ISO/IEC 14496-12 does not allow
a track_ID of 0.
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 311c1d3..02db6ac 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -45,6 +45,10 @@
* Support a 'thmb', 'auxl', 'cdsc' or 'prem' item reference that links one item
to several items. Only the target parsed last used to be kept, silently
dropping the others.
+* Support a track 'auxl' or 'prem' reference that names several tracks, and
+ several such boxes in one 'tref'. Only the first track_ID of a box was read,
+ and a later box overwrote what an earlier one recorded, so the alpha track of
+ an image sequence could be dropped without a diagnostic.
* Keep the premultiplied alpha flag of a MinimizedImageBox. The one bit
alpha_is_premultiplied value was stored in a field compared against the alpha
item ID, so such an image used to decode as straight alpha.
diff --git a/src/read.c b/src/read.c
index e921369..3dc988b 100644
--- a/src/read.c
+++ b/src/read.c
@@ -252,21 +252,21 @@
} avifDecoderItem;
AVIF_ARRAY_DECLARE(avifDecoderItemArray, avifDecoderItem *, item);
-// Records "this item is a {type} for item #{toID}".
-static avifResult avifDecoderItemAddReference(avifDecoderItem * item, const char * type, uint32_t toID)
+// Records "this item or track is a {type} for #{toID}".
+static avifResult avifAddReference(avifItemReferenceArray * references, const char * type, uint32_t toID)
{
- avifItemReference * reference = (avifItemReference *)avifArrayPush(&item->references);
+ avifItemReference * reference = (avifItemReference *)avifArrayPush(references);
AVIF_CHECKERR(reference != NULL, AVIF_RESULT_OUT_OF_MEMORY);
memcpy(reference->type, type, 4);
reference->toID = toID;
return AVIF_RESULT_OK;
}
-// Returns AVIF_TRUE if this item is a {type} for item #{toID}.
-static avifBool avifDecoderItemReferences(const avifDecoderItem * item, const char * type, uint32_t toID)
+// Returns AVIF_TRUE if this item or track is a {type} for #{toID}.
+static avifBool avifReferences(const avifItemReferenceArray * references, const char * type, uint32_t toID)
{
- for (uint32_t i = 0; i < item->references.count; ++i) {
- const avifItemReference * reference = &item->references.ref[i];
+ for (uint32_t i = 0; i < references->count; ++i) {
+ const avifItemReference * reference = &references->ref[i];
if (!memcmp(reference->type, type, 4) && reference->toID == toID) {
return AVIF_TRUE;
}
@@ -274,11 +274,11 @@
return AVIF_FALSE;
}
-// Returns AVIF_TRUE if this item is a {type} for at least one item.
-static avifBool avifDecoderItemHasReference(const avifDecoderItem * item, const char * type)
+// Returns AVIF_TRUE if this item or track is a {type} for at least one item or track.
+static avifBool avifHasReference(const avifItemReferenceArray * references, const char * type)
{
- for (uint32_t i = 0; i < item->references.count; ++i) {
- if (!memcmp(item->references.ref[i].type, type, 4)) {
+ for (uint32_t i = 0; i < references->count; ++i) {
+ if (!memcmp(references->ref[i].type, type, 4)) {
return AVIF_TRUE;
}
}
@@ -501,8 +501,10 @@
{
uint32_t id;
uint8_t handlerType[4];
- uint32_t auxForID; // if non-zero, this track is an auxC plane for Track #{auxForID}
- uint32_t premByID; // if non-zero, this track is premultiplied by Track #{premByID}
+ // The 'auxl' and 'prem' references from this track. Section 8.3.3.1 of ISO/IEC 14496-12
+ // says each reference type shall occur at most once, but that is not enforced for backward
+ // compatibility, and a box can name several track_IDs, so one track can hold several of these.
+ avifItemReferenceArray references;
uint32_t mediaTimescale;
uint64_t mediaDuration;
uint64_t trackDuration;
@@ -1128,6 +1130,11 @@
avifArrayPop(&data->tracks);
return NULL;
}
+ if (!avifArrayCreate(&track->references, sizeof(avifItemReference), 1)) {
+ avifMetaDestroy(track->meta);
+ avifArrayPop(&data->tracks);
+ return NULL;
+ }
return track;
}
@@ -1179,6 +1186,7 @@
if (track->meta) {
avifMetaDestroy(track->meta);
}
+ avifArrayDestroy(&track->references);
}
avifArrayDestroy(&data->tracks);
avifDecoderDataClearTiles(data);
@@ -1947,7 +1955,7 @@
continue;
}
- if ((colorId > 0) && !avifDecoderItemReferences(item, "cdsc", colorId)) {
+ if ((colorId > 0) && !avifReferences(&item->references, "cdsc", colorId)) {
// Not a content description (metadata) for the colorOBU, skip it
continue;
}
@@ -3446,7 +3454,7 @@
// The items linked to are then represented by an array of to_item_IDs.
// Several boxes of the same type may also share a from_item_ID, so an item can
// be the source of more than one reference of a given type. Every target is kept.
- AVIF_CHECKRES(avifDecoderItemAddReference(item, (const char *)irefHeader.type, toID));
+ AVIF_CHECKRES(avifAddReference(&item->references, (const char *)irefHeader.type, toID));
} else if (!memcmp(irefHeader.type, "dimg", 4)) {
// derived images refer in the opposite direction
avifDecoderItem * dimg;
@@ -3910,18 +3918,26 @@
avifBoxHeader header;
AVIF_CHECK(avifROStreamReadBoxHeader(&s, &header));
- if (!memcmp(header.type, "auxl", 4)) {
+ if (!memcmp(header.type, "auxl", 4) || !memcmp(header.type, "prem", 4)) {
+ // Refuse a box that holds no track_ID at all.
AVIF_CHECK(header.size >= sizeof(uint32_t));
- uint32_t toID;
- AVIF_CHECK(avifROStreamReadU32(&s, &toID)); // unsigned int(32) track_IDs[];
- AVIF_CHECK(avifROStreamSkip(&s, header.size - sizeof(uint32_t))); // just take the first one
- track->auxForID = toID;
- } else if (!memcmp(header.type, "prem", 4)) {
- AVIF_CHECK(header.size >= sizeof(uint32_t));
- uint32_t byID;
- AVIF_CHECK(avifROStreamReadU32(&s, &byID)); // unsigned int(32) track_IDs[];
- AVIF_CHECK(avifROStreamSkip(&s, header.size - sizeof(uint32_t))); // just take the first one
- track->premByID = byID;
+ // Section 8.3.3.2 of ISO/IEC 14496-12:
+ // unsigned int(32) track_IDs[];
+ // There is no count field: the number of track_IDs follows from the box size.
+ // Bytes that do not make up a whole track_ID are skipped.
+ const size_t numTrackIDs = header.size / sizeof(uint32_t);
+ for (size_t i = 0; i < numTrackIDs; ++i) {
+ uint32_t toID;
+ AVIF_CHECK(avifROStreamReadU32(&s, &toID));
+ if (toID == 0) {
+ // Section 8.3.3.3 of ISO/IEC 14496-12:
+ // The value 0 shall not be present.
+ // Skip it instead of rejecting for backward compatibility.
+ continue;
+ }
+ AVIF_CHECK(avifAddReference(&track->references, (const char *)header.type, toID) == AVIF_RESULT_OK);
+ }
+ AVIF_CHECK(avifROStreamSkip(&s, header.size - numTrackIDs * sizeof(uint32_t)));
} else {
AVIF_CHECK(avifROStreamSkip(&s, header.size));
}
@@ -4581,9 +4597,9 @@
if (hasAlpha) {
// Property with fixed index 7.
- AVIF_CHECKRES(avifDecoderItemAddReference(alphaItem, "auxl", colorItem->id));
+ AVIF_CHECKRES(avifAddReference(&alphaItem->references, "auxl", colorItem->id));
if (alphaIsPremultiplied) {
- AVIF_CHECKRES(avifDecoderItemAddReference(colorItem, "prem", alphaItem->id));
+ AVIF_CHECKRES(avifAddReference(&colorItem->references, "prem", alphaItem->id));
}
avifProperty * alphaAuxProp = avifMetaCreateProperty(meta, "auxC");
AVIF_CHECKERR(alphaAuxProp, AVIF_RESULT_OUT_OF_MEMORY);
@@ -4808,7 +4824,7 @@
avifDecoderItem * exifItem;
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/6, &exifItem));
memcpy(exifItem->type, "Exif", 4);
- AVIF_CHECKRES(avifDecoderItemAddReference(exifItem, "cdsc", colorItem->id));
+ AVIF_CHECKRES(avifAddReference(&exifItem->references, "cdsc", colorItem->id));
avifExtent * exifExtent = (avifExtent *)avifArrayPush(&exifItem->extents);
AVIF_CHECKERR(exifExtent, AVIF_RESULT_OUT_OF_MEMORY);
@@ -4823,7 +4839,7 @@
AVIF_CHECKRES(avifMetaFindOrCreateItem(meta, /*itemID=*/7, &xmpItem));
memcpy(xmpItem->type, "mime", 4);
memcpy(xmpItem->contentType.contentType, AVIF_CONTENT_TYPE_XMP, sizeof(AVIF_CONTENT_TYPE_XMP));
- AVIF_CHECKRES(avifDecoderItemAddReference(xmpItem, "cdsc", colorItem->id));
+ AVIF_CHECKRES(avifAddReference(&xmpItem->references, "cdsc", colorItem->id));
avifExtent * xmpExtent = (avifExtent *)avifArrayPush(&xmpItem->extents);
AVIF_CHECKERR(xmpExtent, AVIF_RESULT_OUT_OF_MEMORY);
@@ -5343,7 +5359,7 @@
{
return !item->size || item->hasUnsupportedEssentialProperty ||
(avifGetCodecType(item->type) == AVIF_CODEC_TYPE_UNKNOWN && memcmp(item->type, "grid", 4)) ||
- avifDecoderItemHasReference(item, "thmb");
+ avifHasReference(&item->references, "thmb");
}
avifResult avifDecoderParse(avifDecoder * decoder)
@@ -5541,7 +5557,7 @@
// item.
static avifBool avifDecoderItemIsAlphaAux(const avifDecoderItem * item, uint32_t colorItemId)
{
- if (!avifDecoderItemReferences(item, "auxl", colorItemId))
+ if (!avifReferences(&item->references, "auxl", colorItemId))
return AVIF_FALSE;
const avifProperty * auxCProp = avifPropertyArrayFind(&item->properties, "auxC");
return auxCProp && isAlphaURN(auxCProp->u.auxC.auxType);
@@ -5755,7 +5771,7 @@
{
for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) {
avifDecoderItem * item = data->meta->items.item[itemIndex];
- if (!item->size || item->hasUnsupportedEssentialProperty || avifDecoderItemHasReference(item, "thmb")) {
+ if (!item->size || item->hasUnsupportedEssentialProperty || avifHasReference(&item->references, "thmb")) {
continue;
}
if (!memcmp(item->type, "tmap", 4)) {
@@ -6064,7 +6080,7 @@
for (uint32_t itemIndex = 0; itemIndex < data->meta->items.count; ++itemIndex) {
avifDecoderItem * item = data->meta->items.item[itemIndex];
if (!memcmp(item->type, "sato", 4) && item->id != data->meta->primaryItemID && item->size != 0 &&
- !item->hasUnsupportedEssentialProperty && !avifDecoderItemHasReference(item, "thmb") &&
+ !item->hasUnsupportedEssentialProperty && !avifHasReference(&item->references, "thmb") &&
avifIsPreferredAlternativeTo(data, item->id, data->meta->primaryItemID)) {
return item;
}
@@ -6210,7 +6226,7 @@
if (colorCodecType == AVIF_CODEC_TYPE_UNKNOWN) {
continue;
}
- if (track->auxForID != 0) {
+ if (avifHasReference(&track->references, "auxl")) {
continue;
}
// HEIF (ISO/IEC 23008-12:2022), Section 7.1:
@@ -6272,7 +6288,7 @@
// HEIF (ISO/IEC 23008-12:2022), Section 7.5.3.1, but old versions of libavif used to write
// "pict" instead. See https://github.com/AOMediaCodec/libavif/commit/65d0af9
- if (track->auxForID == colorTrack->id) {
+ if (avifReferences(&track->references, "auxl", colorTrack->id)) {
// Found it!
alphaProperties = properties;
break;
@@ -6353,7 +6369,7 @@
decoder->image->width = colorTrack->width;
decoder->image->height = colorTrack->height;
decoder->alphaPresent = (alphaTrack != NULL);
- decoder->image->alphaPremultiplied = decoder->alphaPresent && (colorTrack->premByID == alphaTrack->id);
+ decoder->image->alphaPremultiplied = decoder->alphaPresent && avifReferences(&colorTrack->references, "prem", alphaTrack->id);
} else {
// Create from items
@@ -6502,8 +6518,8 @@
AVIF_CHECKERR(!mainItems[alphaCategory] == !mainItems[AVIF_ITEM_ALPHA], AVIF_RESULT_NOT_IMPLEMENTED);
if (mainItems[alphaCategory] != NULL) {
AVIF_CHECKERR(isAlphaInputImageItemInInput == isAlphaItemInInput, AVIF_RESULT_NOT_IMPLEMENTED);
- AVIF_CHECKERR(avifDecoderItemReferences(mainItems[*category], "prem", mainItems[alphaCategory]->id) ==
- avifDecoderItemReferences(mainItems[AVIF_ITEM_COLOR], "prem", mainItems[AVIF_ITEM_ALPHA]->id),
+ AVIF_CHECKERR(avifReferences(&mainItems[*category]->references, "prem", mainItems[alphaCategory]->id) ==
+ avifReferences(&mainItems[AVIF_ITEM_COLOR]->references, "prem", mainItems[AVIF_ITEM_ALPHA]->id),
AVIF_RESULT_NOT_IMPLEMENTED);
AVIF_CHECKRES(avifDecoderItemReadAndParse(decoder,
mainItems[alphaCategory],
@@ -6604,7 +6620,7 @@
decoder->image->height = mainItems[AVIF_ITEM_COLOR]->height;
decoder->alphaPresent = (mainItems[AVIF_ITEM_ALPHA] != NULL);
decoder->image->alphaPremultiplied =
- decoder->alphaPresent && avifDecoderItemReferences(mainItems[AVIF_ITEM_COLOR], "prem", mainItems[AVIF_ITEM_ALPHA]->id);
+ decoder->alphaPresent && avifReferences(&mainItems[AVIF_ITEM_COLOR]->references, "prem", mainItems[AVIF_ITEM_ALPHA]->id);
if (mainItems[AVIF_ITEM_ALPHA]) {
alphaProperties = &mainItems[AVIF_ITEM_ALPHA]->properties;
diff --git a/tests/gtest/avifanimationtest.cc b/tests/gtest/avifanimationtest.cc
index 22c5fda..5e67583 100644
--- a/tests/gtest/avifanimationtest.cc
+++ b/tests/gtest/avifanimationtest.cc
@@ -2,6 +2,9 @@
// SPDX-License-Identifier: BSD-2-Clause
#include <algorithm>
+#include <cstdint>
+#include <cstring>
+#include <vector>
#include "avif/avif.h"
#include "aviftest_helpers.h"
@@ -98,6 +101,121 @@
AVIF_RESULT_NO_IMAGES_REMAINING);
}
+//------------------------------------------------------------------------------
+// The alpha track of colors-animated-8bpc-alpha-exif-xmp.avif holds a 20 byte
+// 'tref' box at offset 1586, with a single 'auxl' box naming track 1, directly
+// followed by a 44 byte 'edts' box. Section 8.3.3.2 of ISO/IEC 14496-12 gives
+// TrackReferenceTypeBox an array of track_IDs, and nothing limits a 'tref' to
+// one box of a given type. The cases below rewrite those 64 bytes in place and
+// pad with a 'free' box, so the file size and every absolute offset in the file
+// keep the values of the valid file.
+
+constexpr const char* kAlphaTrackFileName =
+ "colors-animated-8bpc-alpha-exif-xmp.avif";
+constexpr size_t kTrefOffset = 1586;
+constexpr size_t kTrefSize = 20;
+constexpr size_t kTrefAndEdtsSize = 64;
+
+void WriteBE32(uint32_t value, uint8_t* bytes) {
+ bytes[0] = static_cast<uint8_t>(value >> 24);
+ bytes[1] = static_cast<uint8_t>(value >> 16);
+ bytes[2] = static_cast<uint8_t>(value >> 8);
+ bytes[3] = static_cast<uint8_t>(value);
+}
+
+void AppendTrackReferenceTypeBox(const char* type,
+ const std::vector<uint32_t>& track_ids,
+ std::vector<uint8_t>* bytes) {
+ const size_t size = 8 + 4 * track_ids.size();
+ const size_t offset = bytes->size();
+ bytes->resize(offset + size);
+ WriteBE32(static_cast<uint32_t>(size), bytes->data() + offset);
+ std::memcpy(bytes->data() + offset + 4, type, 4);
+ for (size_t i = 0; i < track_ids.size(); ++i) {
+ WriteBE32(track_ids[i], bytes->data() + offset + 8 + 4 * i);
+ }
+}
+
+// Overwrites the 'tref' and 'edts' boxes of the alpha track with |boxes| and
+// fills the rest of the 64 bytes with a 'free' box.
+void ReplaceTrefAndEdts(const std::vector<uint8_t>& boxes,
+ testutil::AvifRwData* encoded) {
+ ASSERT_GE(encoded->size, kTrefOffset + kTrefAndEdtsSize);
+ uint8_t* p = encoded->data + kTrefOffset;
+ // Sanity check: the fixture is the file these tests were authored against.
+ ASSERT_EQ(std::memcmp(p + 4, "tref", 4), 0);
+ ASSERT_EQ(std::memcmp(p + kTrefSize + 4, "edts", 4), 0);
+ ASSERT_LE(boxes.size() + 8, kTrefAndEdtsSize);
+ std::copy(boxes.begin(), boxes.end(), p);
+ uint8_t* free_box = p + boxes.size();
+ const size_t free_size = kTrefAndEdtsSize - boxes.size();
+ WriteBE32(static_cast<uint32_t>(free_size), free_box);
+ std::memcpy(free_box + 4, "free", 4);
+ std::memset(free_box + 8, 0, free_size - 8);
+}
+
+void ExpectAlphaTrackIsFound(const testutil::AvifRwData& encoded) {
+ DecoderPtr decoder(avifDecoderCreate());
+ ASSERT_NE(decoder, nullptr);
+ ASSERT_EQ(avifDecoderSetIOMemory(decoder.get(), encoded.data, encoded.size),
+ AVIF_RESULT_OK);
+ ASSERT_EQ(avifDecoderParse(decoder.get()), AVIF_RESULT_OK);
+ EXPECT_EQ(decoder->alphaPresent, AVIF_TRUE);
+ EXPECT_EQ(decoder->imageSequenceTrackPresent, AVIF_TRUE);
+ EXPECT_EQ(decoder->imageCount, 5);
+}
+
+// Control for the two cases below. The alpha track keeps its single reference,
+// only its 'edts' box is replaced by a 'free' box. Dropping it leaves that
+// track's repetition count unknown, which the decoder takes from the color
+// track anyway, so the alpha track still has to be found.
+TEST(AvifDecodeTest, AlphaTrackWithoutEdts) {
+ testutil::AvifRwData encoded =
+ testutil::ReadFile(std::string(data_path) + kAlphaTrackFileName);
+ ASSERT_NE(encoded.size, size_t{0});
+ std::vector<uint8_t> tref;
+ AppendTrackReferenceTypeBox("auxl", {1}, &tref);
+ std::vector<uint8_t> boxes;
+ AppendTrackReferenceTypeBox("tref", {}, &boxes);
+ WriteBE32(static_cast<uint32_t>(8 + tref.size()), boxes.data());
+ boxes.insert(boxes.end(), tref.begin(), tref.end());
+ ASSERT_NO_FATAL_FAILURE(ReplaceTrefAndEdts(boxes, &encoded));
+ ExpectAlphaTrackIsFound(encoded);
+}
+
+// The 'tref' box holds two 'auxl' boxes. Check that both are read.
+TEST(AvifDecodeTest, AlphaTrackWithTwoAuxlBoxes) {
+ testutil::AvifRwData encoded =
+ testutil::ReadFile(std::string(data_path) + kAlphaTrackFileName);
+ ASSERT_NE(encoded.size, size_t{0});
+ std::vector<uint8_t> children;
+ AppendTrackReferenceTypeBox("auxl", {1}, &children);
+ AppendTrackReferenceTypeBox("auxl", {99}, &children);
+ std::vector<uint8_t> boxes;
+ AppendTrackReferenceTypeBox("tref", {}, &boxes);
+ WriteBE32(static_cast<uint32_t>(8 + children.size()), boxes.data());
+ boxes.insert(boxes.end(), children.begin(), children.end());
+ ASSERT_NO_FATAL_FAILURE(ReplaceTrefAndEdts(boxes, &encoded));
+ ExpectAlphaTrackIsFound(encoded);
+}
+
+// One 'auxl' box names two tracks. Check that both are read.
+TEST(AvifDecodeTest, AlphaTrackWithTwoTrackIds) {
+ testutil::AvifRwData encoded =
+ testutil::ReadFile(std::string(data_path) + kAlphaTrackFileName);
+ ASSERT_NE(encoded.size, size_t{0});
+ std::vector<uint8_t> children;
+ AppendTrackReferenceTypeBox("auxl", {99, 1}, &children);
+ std::vector<uint8_t> boxes;
+ AppendTrackReferenceTypeBox("tref", {}, &boxes);
+ WriteBE32(static_cast<uint32_t>(8 + children.size()), boxes.data());
+ boxes.insert(boxes.end(), children.begin(), children.end());
+ ASSERT_NO_FATAL_FAILURE(ReplaceTrefAndEdts(boxes, &encoded));
+ ExpectAlphaTrackIsFound(encoded);
+}
+
+//------------------------------------------------------------------------------
+
TEST(AvifDecodeTest, AnimatedImageWithAlphaAndMetadataIgnoreAlpha) {
const char* file_name = "colors-animated-8bpc-alpha-exif-xmp.avif";
DecoderPtr decoder(avifDecoderCreate());
