Add properties to avifImage

[y-guyon]

No description provided.

[y-guyon]

This is an API design proposition for exposing the primary image item properties that are not used by libavif to the user.

Context: #2395

[bradh]

I think the API is probably fine.

I cannot make the implementation work though - it looks like only supported properties get associated with the image item. I think I got over that, and then hit a double free() on the box contents.

Will send a PR for further review.

[y-guyon]

I cannot make the implementation work though - it looks like only supported properties get associated with the image item. I think I got over that, and then hit a double free() on the box contents.

Thank you for trying it out.

Will send a PR for further review.

I wanted to make sure the API design is accepted by the other maintainers before turning this draft into a proper tested PR.
@wantehchang @vrabaud What do you think about adding this new public API element? Do you see another way of implementing the use case? Do you think of other useful usages of this API design? It is too limited to the primary item? Should it be a fully separate API?

[wantehchang]

Yannis: The current API looks fine. My only suggestion is to anticipate future feature requests (such as image sequence and encoding support) and make sure the API can be extended to support the new features. It would also be good to verify that Mr. Brad Hards is committed to using the new API.

As for the API itself, an alternative would be new functions to iterate over the opaque image item properties of the current image of the decoder (i.e., decoder->image), or a function to look up a given image item property of decoder->image. I am not sure if these alternatives would be better though.

[bradh]

It would also be good to verify that Mr. Brad Hards is committed to using the new API.

I am not sure what commitment looks like, but I am definitely planning to use the API (in whatever form it ends up taking).

Right now I have this branch: https://github.com/bradh/gdal/tree/heif_meta_2024-09-22 which allows GDAL (which is already using libavif) to extract geospatial data.

$ gdalinfo -mdd all  ~/eofff/uncompressed_experiments/geo.avif
Driver: AVIF/AV1 Image File Format
Files: /home/bradh/eofff/uncompressed_experiments/geo.avif
Size is 2048, 1024
Coordinate System is:
PROJCRS["GDA94 / MGA zone 55",
    BASEGEOGCRS["GDA94",
        DATUM["Geocentric Datum of Australia 1994",
            ELLIPSOID["GRS 1980",6378137,298.257222101004,
                LENGTHUNIT["metre",1]]],
        PRIMEM["Greenwich",0,
            ANGLEUNIT["degree",0.0174532925199433]],
        ID["EPSG",4283]],
    CONVERSION["Transverse Mercator",
        METHOD["Transverse Mercator",
            ID["EPSG",9807]],
        PARAMETER["Latitude of natural origin",0,
            ANGLEUNIT["degree",0.0174532925199433],
            ID["EPSG",8801]],
        PARAMETER["Longitude of natural origin",147,
            ANGLEUNIT["degree",0.0174532925199433],
            ID["EPSG",8802]],
        PARAMETER["Scale factor at natural origin",0.9996,
            SCALEUNIT["unity",1],
            ID["EPSG",8805]],
        PARAMETER["False easting",500000,
            LENGTHUNIT["metre",1],
            ID["EPSG",8806]],
        PARAMETER["False northing",10000000,
            LENGTHUNIT["metre",1],
            ID["EPSG",8807]]],
    CS[Cartesian,2],
        AXIS["easting",east,
            ORDER[1],
            LENGTHUNIT["metre",1]],
        AXIS["northing",north,
            ORDER[2],
            LENGTHUNIT["metre",1]],
    ID["EPSG",28355]]
Data axis to CRS axis mapping: 1,2
Origin = (691000.000000001979060,6090000.000000040046871)
Pixel Size = (0.100000000000000,-0.100000000000000)
GCP Projection = 
PROJCRS["GDA94 / MGA zone 55",
    BASEGEOGCRS["GDA94",
        DATUM["Geocentric Datum of Australia 1994",
            ELLIPSOID["GRS 1980",6378137,298.257222101004,
                LENGTHUNIT["metre",1]]],
        PRIMEM["Greenwich",0,
            ANGLEUNIT["degree",0.0174532925199433]],
        ID["EPSG",4283]],
    CONVERSION["Transverse Mercator",
        METHOD["Transverse Mercator",
            ID["EPSG",9807]],
        PARAMETER["Latitude of natural origin",0,
            ANGLEUNIT["degree",0.0174532925199433],
            ID["EPSG",8801]],
        PARAMETER["Longitude of natural origin",147,
            ANGLEUNIT["degree",0.0174532925199433],
            ID["EPSG",8802]],
        PARAMETER["Scale factor at natural origin",0.9996,
            SCALEUNIT["unity",1],
            ID["EPSG",8805]],
        PARAMETER["False easting",500000,
            LENGTHUNIT["metre",1],
            ID["EPSG",8806]],
        PARAMETER["False northing",10000000,
            LENGTHUNIT["metre",1],
            ID["EPSG",8807]]],
    CS[Cartesian,2],
        AXIS["easting",east,
            ORDER[1],
            LENGTHUNIT["metre",1]],
        AXIS["northing",north,
            ORDER[2],
            LENGTHUNIT["metre",1]],
    ID["EPSG",28355]]
Data axis to CRS axis mapping: 1,2
GCP[  0]: Id=0, Info=
          (0,0) -> (691000.000000002,6090000.00000004,0)
Metadata (GIMI):
  ITEM_CONTENT_ID=40a8376-a40-41b3-a7ee-d91760e8db83
Metadata (DESCRIPTION_en-AU):
  NAME=Copyright Statement
  DESCRIPTION=CCBY "Jacobs Group (Australia) Pty Ltd and Australian Capital Territory"
  TAGS=copyright
Metadata (TIMING):
  TIME_UNCERTAINTY=86400000000000 ns
  CLOCK_RESOLUTION=1000 ns
  CLOCK_DRIFT_RATE=(Unknown)
  CLOCK_TYPE=0 (Unknown)
  TAI_TIMESTAMP=1872806437000000000 ns
  SYNCHRONIZATION_STATE=NOT SYNCHRONIZED
  TIMESTAMP_GENERATION_FAILURE=NO
  TIMESTAMP_IS_MODIFIED=NO
Metadata (DERIVED_SUBDATASETS):
  DERIVED_SUBDATASET_1_NAME=DERIVED_SUBDATASET:LOGAMPLITUDE:/home/bradh/eofff/uncompressed_experiments/geo.avif
  DERIVED_SUBDATASET_1_DESC=log10 of amplitude of input bands from /home/bradh/eofff/uncompressed_experiments/geo.avif
Image Structure Metadata:
  YUV_SUBSAMPLING=420
Corner Coordinates:
Upper Left  (  691000.000, 6090000.000) (149d 6' 3.74"E, 35d18'53.96"S)
Lower Left  (  691000.000, 6089897.600) (149d 6' 3.83"E, 35d18'57.28"S)
Upper Right (  691204.800, 6090000.000) (149d 6'11.85"E, 35d18'53.82"S)
Lower Right (  691204.800, 6089897.600) (149d 6'11.93"E, 35d18'57.14"S)
Center      (  691102.400, 6089948.800) (149d 6' 7.84"E, 35d18'55.55"S)
Band 1 Block=2048x1 Type=Byte, ColorInterp=Red
Band 2 Block=2048x1 Type=Byte, ColorInterp=Green
Band 3 Block=2048x1 Type=Byte, ColorInterp=Blue

The sample file:
geo.avif.zip

The spatial extensions are not yet finalised (depends on OGC standards process), but the Content ID (which uses uuid box) is close to NGA standardisation (probably early 2025), and the timing boxes are in FDIS for ISO/IEC 23001-17 Amd 1.
The user description is existing udes from ISO/IEC 23008-12.

[bradh]

Note that the branch is still WIP, and I'll generalise the implementation to share the parsing code with the equivalent functionality in libheif.

[y-guyon]

circle_custom_properties.avif generation snippet

Code added to avifRWStreamWriteProperties() to generate tests/data/circle_custom_properties.avif:

avifItemPropertyDedupStart(dedup);
avifBoxMarker custom1234;
AVIF_CHECKRES(avifRWStreamWriteFullBox(&dedup->s, "1234", AVIF_BOX_SIZE_TBD, 0, 0, &custom1234));
AVIF_CHECKRES(avifRWStreamWriteU8(&dedup->s, 1));
AVIF_CHECKRES(avifRWStreamWriteU8(&dedup->s, 2));
AVIF_CHECKRES(avifRWStreamWriteU8(&dedup->s, 3));
AVIF_CHECKRES(avifRWStreamWriteU8(&dedup->s, 4));
avifRWStreamFinishBox(&dedup->s, custom1234);
AVIF_CHECKRES(avifItemPropertyDedupFinish(dedup, s, &item->ipma, AVIF_FALSE));

avifItemPropertyDedupStart(dedup);
avifBoxMarker customabcd;
AVIF_CHECKRES(avifRWStreamWriteBox(&dedup->s, "abcd", AVIF_BOX_SIZE_TBD, &customabcd));
AVIF_CHECKRES(avifRWStreamWriteChars(&dedup->s, "abcd", 5));
avifRWStreamFinishBox(&dedup->s, customabcd);
AVIF_CHECKRES(avifItemPropertyDedupFinish(dedup, s, &item->ipma, AVIF_FALSE));

avifItemPropertyDedupStart(dedup);
avifBoxMarker customuuid;
AVIF_CHECKRES(avifRWStreamWriteBox(&dedup->s, "uuid", AVIF_BOX_SIZE_TBD, &customuuid));
AVIF_CHECKRES(avifRWStreamWriteChars(&dedup->s, "extended_type 16", 16));
avifRWStreamFinishBox(&dedup->s, customuuid);
AVIF_CHECKRES(avifItemPropertyDedupFinish(dedup, s, &item->ipma, AVIF_FALSE));

[y-guyon]

Yannis: The current API looks fine. My only suggestion is to anticipate future feature requests (such as image sequence and encoding support) and make sure the API can be extended to support the new features.

Potential future features:

• image sequence support
  • This can be done the same way as pasp, carried over in avifDecoder::image for each frame (or not).
• encoding support
  • This can be done easily by no longer ignoring avifImage::properties at encoding. Can be associated with the primary image item and the gainmap. Not sure about the tmap though, I guess it can be associated with both.
• other items than primary and gain map
  • avifImageItemProperty is not used as is by the API, it is only dynamically allocated as an array in avifImage. I imagine it is feasible to append fields to the avifImageItemProperty struct without breaking the API (but it is ABI-breaking, right?).

As for the API itself, an alternative would be new functions to iterate over the opaque image item properties of the current image of the decoder (i.e., decoder->image), or a function to look up a given image item property of decoder->image. I am not sure if these alternatives would be better though.

These would be on top of the currently suggested API, right? Then I am also not sure if it is better.

Alternatives considered:

• Getters directly on avifDecoder rather than modifying the avifImage struct. This is likely cumbersome to libavif users but probably easier to extend or deprecate. It also makes less sense that keeping them close to other supported image properties such as clli. It may be possible to save memory this way too, by not copying bytes if we have access to a byte array as input (instead of a stream).
  API example:
  • avifResult avifDecoderGetNumProperties(const avifDecoder* decoder, size_t* numProperties)
  • avifResult avifDecoderGetPropertyType(const avifDecoder* decoder, uint8_t numProperties[4])
  • avifResult avifDecoderGetPropertyUsertype(const avifDecoder* decoder, uint8_t numProperties[16])
  • avifResult avifDecoderGetPropertyPayload(const avifDecoder* decoder, avifROData * bytes)
• Referencing the position of the unsupported properties in the input bitstream rather than copying bytes. This would save memory but is inconvenient to use with streamed input.

Remaining questions:

• Add a field avifBool rememberUnrecognizedProperties to avifDecoder that is false by default. This would save memory in the common case where the user does not care about avifImage::properties. In the other hand it makes the API slightly more complex and I do not expect that many files to have unrecognized properties as of today.

[bradh]

I'm not sure what is required to move this forward (beyond the inevitable "more time in the day"), but I can say that the proposed API meets my needs.

It would be useful if there was encoding support. I can potentially add that if it would help with the hours in the day problem. Image sequence support might be useful, but I don't need that yet.

I don't need custom props on the alpha, gainmap, tmap etc. From an API user perspective, those feel like internal transformative things on the image, so as long as the library is making the magic happen, I'm probably not that concerned with any non-essential properties on those items. I do see why you might want to have a general solution, of course.

In terms of the alternatives, I like the properties as part of the image (rather than the decoder). That also probably makes the encoding support simpler too. Similar considerations on just having the bitstream location. I could adapt to any of the alternatives though - its a weak preference.

I'm neutral on whether there is a flag to turn off the decoding. I do not see an issue to add one function call to the process. I can possibly implement that too if it would help.

[maryla-uc]

LGTM, mostly nits on comments

[wantehchang]

Yannis: I only reviewed the public header avif.h. (No need to wait for my full review with Maryla's approval.) The changes to avif.h look good. Please add an entry to the changelog. Thanks!

[wantehchang]

LGTM. Thanks! You can ignore the suggestions marked as optional.