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 23C90406A6 for ; Wed, 22 Dec 2021 13:49:00 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id CD28368B08C; Wed, 22 Dec 2021 15:48:58 +0200 (EET) Received: from mail-yb1-f174.google.com (mail-yb1-f174.google.com [209.85.219.174]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 9026D68A412 for ; Wed, 22 Dec 2021 15:48:52 +0200 (EET) Received: by mail-yb1-f174.google.com with SMTP id j2so6771723ybg.9 for ; Wed, 22 Dec 2021 05:48:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=mnRu2SauZ8Jq35O2teqnT9YY0+4KloStF8QE9RM0KWM=; b=DNNnG6WhZE0GIvoPIMDugDLfTocBFFQ8e9r1qz7klZOwQy3+7lFzc63H9OBca1NVlu KBYPfOdA/cFbBIVC+PxbTuyfHng6+Hmc7kIn6mW7zqd6QE73do1dr66cpy8PPeMR1MKA fOQi3XCsVVy11mM5sXXsa7LojUwkgtxdIQjALfYGA+wmHi/Mw6lEFcv9HKhr+MBxQYyq ivamgvzR5opAas2vntt07qLqtNkMFQ+xGv7dH/oEXVFZXvB5KX9juEvK0yUGfQfRo+4f inr325kHMo1liXWulLMM5qlUjhX7MFiiL8k+jODcGlwPLBRFgCb0FNYeh/foCiK/2bKA 9CvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=mnRu2SauZ8Jq35O2teqnT9YY0+4KloStF8QE9RM0KWM=; b=Y4vpqn6VlRD7NgzF2BvVNjr9A67Hqp3OmdSu4POEp0UJ2nrRzyKnZQP+2CSimHWgn3 B1CmuI3FdDbbjpwmb1TmfnwwrnlLS+Th7Az7ZvMnen/fN40vTt2jLKNiWBXXhn0nTCCP diuDnMdbaKep/PyB6SL4QJ+TDSaZpmc1bKb1V3UO+Vo1nfSm5HZKfKj2gCKseuxzYAnF w2H66MHe7v/9pT7tBvnh+SFIS6NFY3b/fAHY1e9sl8jBaSWPXJlU64hNjPyIyDBUgpJt tO/v5pBZGzvUJ6wQ81LwTdri/g2H40ElQYHPmgIhWQl/Tw21t7l99+Hx5JlZza9RO4if 4cVA== X-Gm-Message-State: AOAM532gP6fyshndVUhcYBXtQQz8y4QEOqAwB0zu3osFommFZcEGxC1r W4zmFsgdzmU6mGjFkeIQCcK/w+4nmCpv87AFDFGUQSiy X-Google-Smtp-Source: ABdhPJwqhULPgbHJhbcdgOefoUsgnO9ZS5lUUH6kEL/U5jt+4Gc83JECff2Y8rwbYICp4+lVpb1osi3dtRJupxJ5gLM= X-Received: by 2002:a25:4213:: with SMTP id p19mr4268807yba.41.1640180930834; Wed, 22 Dec 2021 05:48:50 -0800 (PST) MIME-Version: 1.0 References: <20211220134420.69078-1-ffmpeg@haasn.xyz> <20211220153122.54839-1-ffmpeg@haasn.xyz> In-Reply-To: From: Hendrik Leppkes Date: Wed, 22 Dec 2021 14:48:39 +0100 Message-ID: To: FFmpeg development discussions and patches Subject: Re: [FFmpeg-devel] [PATCH v8 1/6] lavu/frame: Add Dolby Vision metadata side data type 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 Wed, Dec 22, 2021 at 2:33 PM Andreas Rheinhardt wrote: > > Hendrik Leppkes: > > On Wed, Dec 22, 2021 at 2:00 PM Lynne wrote: > >> > >> 20 Dec 2021, 16:31 by ffmpeg@haasn.xyz: > >> > >>> From: Niklas Haas > >>> > >>> Signed-off-by: Niklas Haas > >>> --- > >>> doc/APIchanges | 3 + > >>> libavutil/dovi_meta.c | 12 ++++ > >>> libavutil/dovi_meta.h | 143 ++++++++++++++++++++++++++++++++++++++++++ > >>> libavutil/frame.c | 1 + > >>> libavutil/frame.h | 9 ++- > >>> libavutil/version.h | 2 +- > >>> 6 files changed, 168 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/doc/APIchanges b/doc/APIchanges > >>> index 17aa664ca3..ff78edec88 100644 > >>> --- a/doc/APIchanges > >>> +++ b/doc/APIchanges > >>> @@ -14,6 +14,9 @@ libavutil: 2021-04-27 > >>> > >>> API changes, most recent first: > >>> > >>> +2021-12-xx - xxxxxxxxxx - lavu 57.12.100 - frame.h > >>> + Add AV_FRAME_DATA_DOVI_METADATA. > >>> + > >>> 2021-12-xx - xxxxxxxxxx - lavf 59.10.100 - avformat.h > >>> Add AVFormatContext io_close2 which returns an int > >>> > >>> diff --git a/libavutil/dovi_meta.c b/libavutil/dovi_meta.c > >>> index 7bd08f6c54..60b4cb2376 100644 > >>> --- a/libavutil/dovi_meta.c > >>> +++ b/libavutil/dovi_meta.c > >>> @@ -33,3 +33,15 @@ AVDOVIDecoderConfigurationRecord *av_dovi_alloc(size_t *size) > >>> > >>> return dovi; > >>> } > >>> + > >>> +AVDOVIMetadata *av_dovi_metadata_alloc(size_t *size) > >>> +{ > >>> + AVDOVIMetadata *dovi = av_mallocz(sizeof(AVDOVIMetadata)); > >>> + if (!dovi) > >>> + return NULL; > >>> + > >>> + if (size) > >>> + *size = sizeof(*dovi); > >>> + > >>> + return dovi; > >>> +} > >>> diff --git a/libavutil/dovi_meta.h b/libavutil/dovi_meta.h > >>> index 299911d434..25e6d7b42f 100644 > >>> --- a/libavutil/dovi_meta.h > >>> +++ b/libavutil/dovi_meta.h > >>> @@ -29,6 +29,7 @@ > >>> > >>> #include > >>> #include > >>> +#include "rational.h" > >>> > >>> /* > >>> * DOVI configuration > >>> @@ -67,4 +68,146 @@ typedef struct AVDOVIDecoderConfigurationRecord { > >>> */ > >>> AVDOVIDecoderConfigurationRecord *av_dovi_alloc(size_t *size); > >>> > >>> +/** > >>> + * Dolby Vision RPU data header. > >>> + * > >>> + * @note Cannot be extended without an ABI bump. > >>> + */ > >>> +typedef struct AVDOVIRpuDataHeader { > >>> + uint8_t rpu_type; > >>> + uint16_t rpu_format; > >>> + uint8_t vdr_rpu_profile; > >>> + uint8_t vdr_rpu_level; > >>> + uint8_t chroma_resampling_explicit_filter_flag; > >>> + uint8_t coef_data_type; /* informative, lavc always converts to fixed */ > >>> + uint8_t coef_log2_denom; > >>> + uint8_t vdr_rpu_normalized_idc; > >>> + uint8_t bl_video_full_range_flag; > >>> + uint8_t bl_bit_depth; /* [8, 16] */ > >>> + uint8_t el_bit_depth; /* [8, 16] */ > >>> + uint8_t vdr_bit_depth; /* [8, 16] */ > >>> + uint8_t spatial_resampling_filter_flag; > >>> + uint8_t el_spatial_resampling_filter_flag; > >>> + uint8_t disable_residual_flag; > >>> +} AVDOVIRpuDataHeader; > >>> + > >>> +enum AVDOVIMappingMethod { > >>> + AV_DOVI_MAPPING_POLYNOMIAL = 0, > >>> + AV_DOVI_MAPPING_MMR = 1, > >>> +}; > >>> + > >>> +/** > >>> + * Coefficients of a piece-wise function. The pieces of the function span the > >>> + * value ranges between two adjacent pivot values. > >>> + * > >>> + * @note Cannot be extended without an ABI bump. > >>> + */ > >>> +#define AV_DOVI_MAX_PIECES 8 > >>> +typedef struct AVDOVIReshapingCurve { > >>> + uint8_t num_pivots; /* [2, 9] */ > >>> + uint16_t pivots[AV_DOVI_MAX_PIECES + 1]; /* sorted ascending */ > >>> + enum AVDOVIMappingMethod mapping_idc[AV_DOVI_MAX_PIECES]; > >>> + /* AV_DOVI_MAPPING_POLYNOMIAL */ > >>> + uint8_t poly_order[AV_DOVI_MAX_PIECES]; /* [1, 2] */ > >>> + int64_t poly_coef[AV_DOVI_MAX_PIECES][3]; /* x^0, x^1, x^2 */ > >>> + /* AV_DOVI_MAPPING_MMR */ > >>> + uint8_t mmr_order[AV_DOVI_MAX_PIECES]; /* [1, 3] */ > >>> + int64_t mmr_constant[AV_DOVI_MAX_PIECES]; > >>> + int64_t mmr_coef[AV_DOVI_MAX_PIECES][3/* order - 1 */][7]; > >>> +} AVDOVIReshapingCurve; > >>> + > >>> +enum AVDOVINLQMethod { > >>> + AV_DOVI_NLQ_NONE = -1, > >>> + AV_DOVI_NLQ_LINEAR_DZ = 0, > >>> +}; > >>> + > >>> +/** > >>> + * Coefficients of the non-linear inverse quantization. For the interpretation > >>> + * of these, see ETSI GS CCM 001. > >>> + * > >>> + * @note Cannot be extended without an ABI bump. > >>> + */ > >>> +typedef struct AVDOVINLQParams { > >>> + uint64_t nlq_offset; > >>> + uint64_t vdr_in_max; > >>> + /* AV_DOVI_NLQ_LINEAR_DZ */ > >>> + uint64_t linear_deadzone_slope; > >>> + uint64_t linear_deadzone_threshold; > >>> +} AVDOVINLQParams; > >>> + > >>> +/** > >>> + * Dolby Vision RPU data mapping parameters. > >>> + * > >>> + * @note Cannot be extended without an ABI bump. > >>> + */ > >>> +typedef struct AVDOVIDataMapping { > >>> + uint8_t vdr_rpu_id; > >>> + uint8_t mapping_color_space; > >>> + uint8_t mapping_chroma_format_idc; > >>> + AVDOVIReshapingCurve curves[3]; /* per component */ > >>> + > >>> + /* Non-linear inverse quantization */ > >>> + enum AVDOVINLQMethod nlq_method_idc; > >>> + uint32_t num_x_partitions; > >>> + uint32_t num_y_partitions; > >>> + AVDOVINLQParams nlq[3]; /* per component */ > >>> +} AVDOVIDataMapping; > >>> + > >>> +typedef struct AVDOVIColorMetadata { > >>> + uint8_t dm_metadata_id; > >>> + uint8_t scene_refresh_flag; > >>> + > >>> + /** > >>> + * Coefficients of the custom Dolby Vision IPT-PQ matrices. These are to be > >>> + * used instead of the matrices indicated by the frame's colorspace tags. > >>> + * The output of rgb_to_lms_matrix is to be fed into a BT.2020 LMS->RGB > >>> + * matrix based on a Hunt-Pointer-Estevez transform, but without any > >>> + * crosstalk. (See the definition of the ICtCp colorspace for more > >>> + * information.) > >>> + */ > >>> + AVRational ycc_to_rgb_matrix[9]; /* before PQ linearization */ > >>> + AVRational ycc_to_rgb_offset[3]; /* input offset of neutral value */ > >>> + AVRational rgb_to_lms_matrix[9]; /* after PQ linearization */ > >>> + > >>> + /** > >>> + * Extra signal metadata (see Dolby patents for more info). > >>> + */ > >>> + uint16_t signal_eotf; > >>> + uint16_t signal_eotf_param0; > >>> + uint16_t signal_eotf_param1; > >>> + uint32_t signal_eotf_param2; > >>> + uint8_t signal_bit_depth; > >>> + uint8_t signal_color_space; > >>> + uint8_t signal_chroma_format; > >>> + uint8_t signal_full_range_flag; /* [0, 3] */ > >>> + uint16_t source_min_pq; > >>> + uint16_t source_max_pq; > >>> + uint16_t source_diagonal; > >>> +} AVDOVIColorMetadata; > >>> + > >>> +/** > >>> + * Combined struct representing a combination of header, mapping and color > >>> + * metadata, for attaching to frames as side data. > >>> + * > >>> + * @note The struct must be allocated with av_dovi_metadata_alloc() and > >>> + * its size is not a part of the public ABI. > >>> + */ > >>> + > >>> +typedef struct AVDOVIMetadata { > >>> + AVDOVIRpuDataHeader header; > >>> + AVDOVIDataMapping mapping; > >>> + AVDOVIColorMetadata color; > >>> > >> > >> I think we should version this by adding an integer version > >> field, and a compile-time header #define. > >> just in case more extra signalling is added. Then > >> we could support it in a sort of backwards-compatible way > >> by just documenting what has changed, and users could > >> make their own decisions about whether to present it > >> as-is or error out. > >> > > > > That seems rather pointless to me when API users can just tie that to > > avutil or avcodec version. > > Additionally, how would I ever make use of that? We never support > > running binaries older then the headers you have, and newer data can't > > be used because you don't have the struct definition that corresponds > > with it. > > > > One way to make these structs extensible while keeping this side data a > single buffer is by adding the offsets of header, mapping and color to > the start of AVDOVIMetadata at the start of AVDOVIMetadata. That way > users could use the parts they know and ignore the rest. It would even > allow for adding further structs to AVDOVIMetadata. > How would you define the struct then? If we define it as a normal struct and people naively access stuff, it might suddenly point to something else - so if we allow us to change it, then we probably should define it in a way that doesn't even allow the naive accidental mis-reading. Keeping that in mind then, it sounds like it would get rather complicated to use. - Hendrik _______________________________________________ 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".