Keep every target of a 'thmb', 'auxl', 'cdsc' or 'prem' reference

[Nexory]

A SingleItemTypeReferenceBox links one item to an array of to_item_IDs
(ISO/IEC 14496-12, section 8.11.12.1). libavif stored the 'thmb', 'auxl',
'cdsc' and 'prem' relations in a single-valued field, and the parsing loop
overwrote it per entry, so the target parsed last silently won.

Measured on files differing from a working one only in the order of two
to_item_ID values, the Exif item describing items [1, 2]:

cdsc 3 -> [1]      Exif Metadata : Present (14 bytes)   exit 0
cdsc 3 -> [1, 2]   Exif Metadata : Absent               exit 0
cdsc 3 -> [2, 1]   Exif Metadata : Present (14 bytes)   exit 0

The decoded pixels are identical in all three, only the Exif payload is gone. For
'auxl' the effect is heavier: with the alpha item declared auxiliary for
[1, 3], the alpha plane is dropped and the image decodes as opaque, again with
exit code 0 and no diagnostic. Two boxes of one type sharing a from_item_ID
behave the same.

This keeps every target instead. The references live in a per item array, the
parsing loop pushes rather than overwrites, and the read sites became
membership checks. 'dimg' is unchanged: it refers in the opposite direction and
already refuses what it cannot represent.

Without the change both tests fail with the Exif data and the alpha plane
missing, while the decode itself still returns AVIF_RESULT_OK.

A second commit fixes the MinimizedImageBox path, which stored the one bit
alpha_is_premultiplied value in the same field. Under the array introduced here
that is a reference to item 1, the primary item itself, so a premultiplied image
decoded as straight alpha. The parameterized test in avifminitest.cc already
compares a MinimizedImageBox against a regular MetaBox and compares
alphaPremultiplied, but no parameter set marks the alpha as premultiplied, so
the flag was never exercised. The added test does.

Two limits remain. I found no encoder that writes such files (libheif passes a
single ID at every call site), so both test files are hand-crafted. I built with
dav1d and aom from the system, rav1e, svt and libgav1 off, not the five codecs
the CI builds from source.

[y-guyon]

Thank you for your interest in libavif.

I am not so sure that rejecting files is a better behavior. How about fixing the issue instead?

[Nexory]

Fair point, and I think you are right.

I checked how the four relations are consumed. Across the tree they appear only
in src/read.c, and the eight read sites on avifDecoderItem never ask for "the"
target, only whether one specific item is among them; thumbnailForID is just
compared against 0. The avifTrack fields of the same name are separate.

So the fields can become id arrays, the loop pushes instead of overwriting, and
those comparisons become a membership check.

Happy to rework the PR that way, with the two tests asserting that the metadata
and the alpha plane survive instead of asserting a rejection. In this PR, or a
fresh one?

[y-guyon]

Happy to rework the PR that way, with the two tests asserting that the metadata and the alpha plane survive instead of asserting a rejection. In this PR, or a fresh one?

As you wish!

[Nexory]

Reworked as suggested.

The four single-valued fields became one per item array of (type, toID)
entries. The parsing loop pushes instead of overwriting, and the read sites
became membership checks. 'dimg' is untouched: it refers in the opposite
direction and already refuses what it cannot represent.

Both tests now assert survival instead of rejection. The Exif metadata is
present for a 'cdsc' naming items [1, 2], and the alpha plane is there for an
'auxl' naming [1, 3].

One thing I did not change: in the MinimizedImageBox path, the line that used
to read

colorItem->premByID = alphaIsPremultiplied;

stores a one bit flag in a field the consumer compares against the alpha item
ID, which is fixed at 2 in that path. I translated it unchanged rather than
fixing it here. Happy to file that separately if you want it looked at.

[Nexory]

I measured the MinimizedImageBox line I mentioned, and folded the fix into this
PR after all, because it turned out not to be a separate issue.

Under the array this PR introduces, toID is an item ID. The old line stored the
one bit alpha_is_premultiplied value, and the primary item ID in that path is 1,
so translating it faithfully wrote "item 1 is premultiplied by item 1" into the
new structure. Keeping that seemed worse than fixing it.

The behaviour on main, measured with avifenc on one input image: encoded with
-p it decodes as premultiplied through a regular MetaBox, and as straight alpha
through --mini. Changing only the reader and decoding the same files restores
it, while an image encoded without -p stays unpremultiplied either way.

The parameterized test in avifminitest.cc already compares a MinimizedImageBox
against a regular MetaBox for the same image, and AreImagesEqual compares
alphaPremultiplied. No parameter set marks the alpha as premultiplied, so the
flag was never exercised. The added test does, and it fails without the change.

Happy to split it back out if you would rather review the two separately.