From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id C57B64A1C1 for ; Sat, 23 Mar 2024 18:08:12 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id E081C68D3CA; Sat, 23 Mar 2024 20:08:10 +0200 (EET) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 9D39F68D1B3 for ; Sat, 23 Mar 2024 20:08:04 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1711217284; bh=42x8sNQG4GK/sA0AFyuckXyLywlj0cABWyFFo4H7EAE=; h=Date:From:To:Subject:In-Reply-To:References:From; b=NbSkANMZxRbBQpJj9Yz4jpInwJKAVc7kkjoPuSEAdW6LDLL9uDq80WOeu0R+JKCuA gBFG/++JA9FPHAGBj7Ba5V4/qQrYMXgDaCbMYb1jUPJokILeCg4O+6DIZLuoIIZHzp I4N2uKuiCKQXozKD/7EGHa3/f815exXkDuWpObd4= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id 6ECD440015 for ; Sat, 23 Mar 2024 19:08:04 +0100 (CET) Date: Sat, 23 Mar 2024 19:08:04 +0100 Message-ID: <20240323190804.GD56135@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: References: <20240323173735.26224-1-ffmpeg@haasn.xyz> <20240323173735.26224-2-ffmpeg@haasn.xyz> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 2/6] avutil/dovi_meta: add dolby vision extension blocks X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On Sat, 23 Mar 2024 18:58:51 +0100 Andreas Rheinhardt wrote: > Niklas Haas: > > From: Niklas Haas > > > > 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 > > + > > #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 > > #include > > + > > #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? > > > #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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".