[FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks
Niklas Haas
ffmpeg at haasn.xyz
Sat Mar 23 20:45:43 EET 2024
On Sat, 23 Mar 2024 19:08:04 +0100 Niklas Haas <ffmpeg at haasn.xyz> wrote:
> On Sat, 23 Mar 2024 18:58:51 +0100 Andreas Rheinhardt <andreas.rheinhardt at outlook.com> wrote:
> > Niklas Haas:
> > > From: Niklas Haas <git at haasn.dev>
> > >
> > > As well as accessors plus a function for allocating this struct with
> > > extension blocks,
> > >
> > > Definitions generously taken from quietvoid/dovi_tool, which is
> > > assembled as a collection of various patent fragments, as well as output
> > > by the official Dolby Vision bitstream verifier tool.
> > > ---
> > > doc/APIchanges | 5 ++
> > > libavutil/dovi_meta.c | 36 ++++++++--
> > > libavutil/dovi_meta.h | 154 ++++++++++++++++++++++++++++++++++++++++++
> > > libavutil/version.h | 2 +-
> > > 4 files changed, 190 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/doc/APIchanges b/doc/APIchanges
> > > index 739f33501e9..8b59150dc1f 100644
> > > --- a/doc/APIchanges
> > > +++ b/doc/APIchanges
> > > @@ -2,6 +2,11 @@ The last version increases of all libraries were on 2024-03-07
> > >
> > > API changes, most recent first:
> > >
> > > +2024-03-xx - xxxxxxxxxx - lavu 59.6.100 - dovi_meta.h
> > > + Add AVDOVIMetadata.ext_block_{offset,size}, AVDOVIMetadata.num_ext_blocks,
> > > + AVDOVIDmData and AVDOVIDmLevel{1..6,8..11,254..255}, av_dovi_get_ext(),
> > > + av_dovi_find_level() and av_dovi_metadata_alloc_ext(),
> > > +
> > > 2024-03-xx - xxxxxxxxxx - lavu 59.5.100 - dovi_meta.h
> > > Add AVDOVIDataMapping.nlq_pivots.
> > >
> > > diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c
> > > index 9c50da561ed..17e18bf95f9 100644
> > > --- a/libavutil/dovi_meta.c
> > > +++ b/libavutil/dovi_meta.c
> > > @@ -18,6 +18,8 @@
> > > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> > > */
> > >
> > > +#include <string.h>
> > > +
> > > #include "dovi_meta.h"
> > > #include "mem.h"
> > >
> > > @@ -39,22 +41,44 @@ typedef struct AVDOVIMetadataInternal {
> > > AVDOVIRpuDataHeader header;
> > > AVDOVIDataMapping mapping;
> > > AVDOVIColorMetadata color;
> > > + AVDOVIDmData ext_blocks[];
> > > } AVDOVIMetadataInternal;
> > >
> > > -AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size)
> > > {
> > > - AVDOVIMetadataInternal *dovi = av_mallocz(sizeof(AVDOVIMetadataInternal));
> > > + const size_t alloc_size = sizeof(AVDOVIMetadataInternal) +
> > > + num_ext_blocks * sizeof(AVDOVIDmData);
> > > +
> > > + AVDOVIMetadataInternal *dovi = av_mallocz(alloc_size);
> > > if (!dovi)
> > > return NULL;
> > >
> > > if (size)
> > > - *size = sizeof(*dovi);
> > > + *size = alloc_size;
> > >
> > > dovi->metadata = (struct AVDOVIMetadata) {
> > > - .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > > - .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > > - .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > > + .header_offset = offsetof(AVDOVIMetadataInternal, header),
> > > + .mapping_offset = offsetof(AVDOVIMetadataInternal, mapping),
> > > + .color_offset = offsetof(AVDOVIMetadataInternal, color),
> > > + .ext_block_offset = offsetof(AVDOVIMetadataInternal, ext_blocks),
> > > + .ext_block_size = sizeof(AVDOVIDmData),
> > > };
> > >
> > > return &dovi->metadata;
> > > }
> > > +
> > > +AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size)
> > > +{
> > > + return av_dovi_metadata_alloc_ext(0, size);
> > > +}
> > > +
> > > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level)
> > > +{
> > > + for (int i = 0; i < data->num_ext_blocks; i++) {
> > > + AVDOVIDmData *ext = av_dovi_get_ext(data, i);
> > > + if (ext->level == level)
> > > + return ext;
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h
> > > index 6afc7b055a7..e77d9853f67 100644
> > > --- a/libavutil/dovi_meta.h
> > > +++ b/libavutil/dovi_meta.h
> > > @@ -29,7 +29,9 @@
> > >
> > > #include <stdint.h>
> > > #include <stddef.h>
> > > +
> > > #include "rational.h"
> > > +#include "csp.h"
> > >
> > > /*
> > > * DOVI configuration
> > > @@ -187,6 +189,130 @@ typedef struct AVDOVIColorMetadata {
> > > uint16_t source_diagonal;
> > > } AVDOVIColorMetadata;
> > >
> > > +typedef struct AVDOVIDmLevel1 {
> > > + /* Per-frame brightness metadata */
> > > + uint16_t min_pq;
> > > + uint16_t max_pq;
> > > + uint16_t avg_pq;
> > > +} AVDOVIDmLevel1;
> > > +
> > > +typedef struct AVDOVIDmLevel2 {
> > > + /* Usually derived from level 8 (at different levels) */
> > > + uint16_t target_max_pq;
> > > + uint16_t trim_slope;
> > > + uint16_t trim_offset;
> > > + uint16_t trim_power;
> > > + uint16_t trim_chroma_weight;
> > > + uint16_t trim_saturation_gain;
> > > + int16_t ms_weight;
> > > +} AVDOVIDmLevel2;
> > > +
> > > +typedef struct AVDOVIDmLevel3 {
> > > + uint16_t min_pq_offset;
> > > + uint16_t max_pq_offset;
> > > + uint16_t avg_pq_offset;
> > > +} AVDOVIDmLevel3;
> > > +
> > > +typedef struct AVDOVIDmLevel4 {
> > > + uint16_t anchor_pq;
> > > + uint16_t anchor_power;
> > > +} AVDOVIDmLevel4;
> > > +
> > > +typedef struct AVDOVIDmLevel5 {
> > > + /* Active area definition */
> > > + uint16_t left_offset;
> > > + uint16_t right_offset;
> > > + uint16_t top_offset;
> > > + uint16_t bottom_offset;
> > > +} AVDOVIDmLevel5;
> > > +
> > > +typedef struct AVDOVIDmLevel6 {
> > > + /* Static HDR10 metadata */
> > > + uint16_t max_luminance;
> > > + uint16_t min_luminance;
> > > + uint16_t max_cll;
> > > + uint16_t max_fall;
> > > +} AVDOVIDmLevel6;
> > > +
> > > +typedef struct AVDOVIDmLevel8 {
> > > + /* Extended version of level 2 */
> > > + uint8_t target_display_index;
> > > + uint16_t trim_slope;
> > > + uint16_t trim_offset;
> > > + uint16_t trim_power;
> > > + uint16_t trim_chroma_weight;
> > > + uint16_t trim_saturation_gain;
> > > + uint16_t ms_weight;
> > > + uint16_t target_mid_contrast;
> > > + uint16_t clip_trim;
> > > + uint8_t saturation_vector_field[6];
> > > + uint8_t hue_vector_field[6];
> > > +} AVDOVIDmLevel8;
> > > +
> > > +typedef struct AVDOVIDmLevel9 {
> > > + /* Source display characteristics */
> > > + uint8_t source_primary_index;
> > > + AVColorPrimariesDesc source_display_primaries;
> > > +} AVDOVIDmLevel9;
> > > +
> > > +typedef struct AVDOVIDmLevel10 {
> > > + /* Target display characteristics */
> > > + uint8_t target_display_index;
> > > + uint16_t target_max_pq;
> > > + uint16_t target_min_pq;
> > > + uint8_t target_primary_index;
> > > + AVColorPrimariesDesc target_display_primaries;
> > > +} AVDOVIDmLevel10;
> > > +
> > > +typedef struct AVDOVIDmLevel11 {
> > > + uint8_t content_type;
> > > + uint8_t whitepoint;
> > > + uint8_t reference_mode_flag;
> > > + uint8_t sharpness;
> > > + uint8_t noise_reduction;
> > > + uint8_t mpeg_noise_reduction;
> > > + uint8_t frame_rate_conversion;
> > > + uint8_t brightness;
> > > + uint8_t color;
> > > +} AVDOVIDmLevel11;
> > > +
> > > +typedef struct AVDOVIDmLevel254 {
> > > + /* DMv2 info block, always present in samples with DMv2 metadata */
> > > + uint8_t dm_mode;
> > > + uint8_t dm_version_index;
> > > +} AVDOVIDmLevel254;
> > > +
> > > +typedef struct AVDOVIDmLevel255 {
> > > + /* Debug block, not really used in samples */
> > > + uint8_t dm_run_mode;
> > > + uint8_t dm_run_version;
> > > + uint8_t dm_debug[4];
> > > +} AVDOVIDmLevel255;
> > > +
> > > +/**
> > > + * Dolby Vision metadata extension block.
> > > + *
> > > + * @note sizeof(AVDOVIDmData) is not part of the public API.
> > > + */
> > > +typedef struct AVDOVIDmData {
> > > + uint8_t level; /* [1, 255] */
> > > + union {
> > > + AVDOVIDmLevel1 l1;
> > > + AVDOVIDmLevel2 l2; /* may appear multiple times */
> > > + AVDOVIDmLevel3 l3;
> > > + AVDOVIDmLevel4 l4;
> > > + AVDOVIDmLevel5 l5;
> > > + AVDOVIDmLevel6 l6;
> > > + /* level 7 is currently unused */
> > > + AVDOVIDmLevel8 l8; /* may appear multiple times */
> > > + AVDOVIDmLevel9 l9;
> > > + AVDOVIDmLevel10 l10; /* may appear multiple times */
> > > + AVDOVIDmLevel11 l11;
> > > + AVDOVIDmLevel254 l254;
> > > + AVDOVIDmLevel255 l255;
> > > + };
> >
> > Unnamed unions are C11
>
> I thought we support C11 now?
>
> >
> > > +} AVDOVIDmData;
> > > +
> > > /**
> > > * Combined struct representing a combination of header, mapping and color
> > > * metadata, for attaching to frames as side data.
> > > @@ -203,6 +329,10 @@ typedef struct AVDOVIMetadata {
> > > size_t header_offset; /* AVDOVIRpuDataHeader */
> > > size_t mapping_offset; /* AVDOVIDataMapping */
> > > size_t color_offset; /* AVDOVIColorMetadata */
> > > +
> > > + size_t ext_block_offset; /* offset to start of ext blocks array */
> > > + size_t ext_block_size; /* size per element */
> > > + int num_ext_blocks; /* number of extension blocks */
> > > } AVDOVIMetadata;
> > >
> > > static av_always_inline AVDOVIRpuDataHeader *
> > > @@ -223,6 +353,19 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > > return (AVDOVIColorMetadata *)((uint8_t *) data + data->color_offset);
> > > }
> > >
> > > +static av_always_inline AVDOVIDmData *
> > > +av_dovi_get_ext(const AVDOVIMetadata *data, int index)
> > > +{
> > > + return (AVDOVIDmData *)((uint8_t *) data + data->ext_block_offset +
> > > + data->ext_block_size * index);
> >
> > This is not const-correct.
>
> It follows the convention of the functions immediately above it. What do
> you suggest? Always returning `const` here is awkward for users who want
> to write to it, but never returning `const` is awkward for users who
> want to read from it.
>
> C really isn't the right tool for the job here..
>
> >
> > > +}
> > > +
> > > +/**
> > > + * Find an extension block with a given level, or NULL. In the case of
> > > + * multiple extension blocks, only the first is returned.
> > > + */
> > > +AVDOVIDmData *av_dovi_find_level(const AVDOVIMetadata *data, uint8_t level);
> > > +
> > > /**
> > > * Allocate an AVDOVIMetadata structure and initialize its
> > > * fields to default values.
> > > @@ -234,4 +377,15 @@ av_dovi_get_color(const AVDOVIMetadata *data)
> > > */
> > > AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size);
> > >
> > > +/**
> > > + * Allocate an AVDOVIMetadata with a given number of extension blocks.
> > > + *
> > > + * @param num_ext_blocks The number of extension blocks to allocate
> > > + * @param size If this parameter is non-NULL, the size in bytes of the
> > > + * allocated struct will be written here on success
> > > + *
> > > + * @return the newly allocated struct or NULL on failure
> > > + */
> > > +AVDOVIMetadata *av_dovi_metadata_alloc_ext(int num_ext_blocks, size_t *size);
> > > +
> >
> > Not another boilerplate allocator.
>
> What do you suggest?
Update: I decided to drop dynamic allocation of this because it turns
out that we're constrained to a maximum of ~32 extension blocks anyway,
and the existing struct size (~5 kB) is already substantially larger
than the size it would take to just always allocate space for 32
extension blocks.
>
> >
> > > #endif /* AVUTIL_DOVI_META_H */
> > > diff --git a/libavutil/version.h b/libavutil/version.h
> > > index 8a1ecd44516..f874a4fd439 100644
> > > --- a/libavutil/version.h
> > > +++ b/libavutil/version.h
> > > @@ -79,7 +79,7 @@
> > > */
> > >
> > > #define LIBAVUTIL_VERSION_MAJOR 59
> > > -#define LIBAVUTIL_VERSION_MINOR 5
> > > +#define LIBAVUTIL_VERSION_MINOR 6
> > > #define LIBAVUTIL_VERSION_MICRO 100
> > >
> > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list