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 9F89F4B5F7 for ; Sun, 9 Jun 2024 15:06:48 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EC0FF68D5AF; Sun, 9 Jun 2024 18:06:10 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 8FE2B68D3D0 for ; Sun, 9 Jun 2024 18:05:58 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1717945557; bh=Z5iayvfcVVjETBAXXwcEHXgso2koMyKWcbGh686N0ow=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LOIEJuywY6n4vbqrJq+UR3app1yw2/kEu0gJV1eEeGy48O0UlljuFLRjiImWLFcms AOwceEad80Li8a8LhSvDoF1gt2oZ4dmIC+wsIpge630fIML4PRj86NheFJQzw4Vbfn mUP19hD8MPx0wJHQ2KkJglV0zwpuLFj2P8uNlalg= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id 78C9941F18; Sun, 9 Jun 2024 17:05:57 +0200 (CEST) From: Niklas Haas To: ffmpeg-devel@ffmpeg.org Date: Sun, 9 Jun 2024 17:05:48 +0200 Message-ID: <20240609150553.72865-3-ffmpeg@haasn.xyz> X-Mailer: git-send-email 2.45.1 In-Reply-To: <20240609150553.72865-1-ffmpeg@haasn.xyz> References: <20240609150553.72865-1-ffmpeg@haasn.xyz> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/8] avcodec/dovi_rpu: fix dm_metadata_id handling 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 Cc: Niklas Haas 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: From: Niklas Haas Despite the suggestive size limits, this metadata ID has nothing to do with the VDR metadata ID used for the data mappings. Actually, the specification leaves them wholly unexplained, other than acknowleding their existence. Must be some secret dolby sauce. They're not even involved in DM metadata compression, which is handled using an entirely separate ID. That leaves us with a lack of anything sensible to do with these IDs. Since we unfortunately only expose one `dm_metadata_id` field to the user, just ensure that they match; which appears to always be the case in practice. If somebody ever hits this error, I would really much rather like to see the triggering file. --- libavcodec/dovi_rpu.c | 3 +++ libavcodec/dovi_rpu.h | 1 + libavcodec/dovi_rpudec.c | 30 +++++++++++++++--------------- libavcodec/dovi_rpuenc.c | 16 ++++++++-------- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/libavcodec/dovi_rpu.c b/libavcodec/dovi_rpu.c index 91134e031d..5130a9598d 100644 --- a/libavcodec/dovi_rpu.c +++ b/libavcodec/dovi_rpu.c @@ -28,6 +28,7 @@ void ff_dovi_ctx_unref(DOVIContext *s) { + ff_refstruct_unref(&s->dm); for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++) ff_refstruct_unref(&s->vdr[i]); ff_refstruct_unref(&s->ext_blocks); @@ -40,6 +41,7 @@ void ff_dovi_ctx_unref(DOVIContext *s) void ff_dovi_ctx_flush(DOVIContext *s) { + ff_refstruct_unref(&s->dm); for (int i = 0; i < FF_ARRAY_ELEMS(s->vdr); i++) ff_refstruct_unref(&s->vdr[i]); ff_refstruct_unref(&s->ext_blocks); @@ -60,6 +62,7 @@ void ff_dovi_ctx_replace(DOVIContext *s, const DOVIContext *s0) s->header = s0->header; s->mapping = s0->mapping; s->color = s0->color; + ff_refstruct_replace(&s->dm, s0->dm); for (int i = 0; i <= DOVI_MAX_DM_ID; i++) ff_refstruct_replace(&s->vdr[i], s0->vdr[i]); ff_refstruct_replace(&s->ext_blocks, s0->ext_blocks); diff --git a/libavcodec/dovi_rpu.h b/libavcodec/dovi_rpu.h index c784afbe4b..da9bd67cde 100644 --- a/libavcodec/dovi_rpu.h +++ b/libavcodec/dovi_rpu.h @@ -74,6 +74,7 @@ typedef struct DOVIContext { /** * Private fields internal to dovi_rpu.c */ + AVDOVIColorMetadata *dm; ///< RefStruct struct DOVIVdr *vdr[DOVI_MAX_DM_ID+1]; ///< RefStruct references uint8_t *rpu_buf; ///< temporary buffer unsigned rpu_buf_sz; diff --git a/libavcodec/dovi_rpudec.c b/libavcodec/dovi_rpudec.c index cf2152988c..3e15453705 100644 --- a/libavcodec/dovi_rpudec.c +++ b/libavcodec/dovi_rpudec.c @@ -568,25 +568,25 @@ int ff_dovi_rpu_parse(DOVIContext *s, const uint8_t *rpu, size_t rpu_size, int current_dm_id = get_ue_golomb_31(gb); VALIDATE(affected_dm_id, 0, DOVI_MAX_DM_ID); VALIDATE(current_dm_id, 0, DOVI_MAX_DM_ID); - if (!s->vdr[affected_dm_id]) { - s->vdr[affected_dm_id] = ff_refstruct_allocz(sizeof(DOVIVdr)); - if (!s->vdr[affected_dm_id]) - return AVERROR(ENOMEM); + if (affected_dm_id != current_dm_id) { + /* The spec does not explain these fields at all, and there is + * a lack of samples to understand how they're supposed to work, + * so just assert them being equal for now */ + avpriv_request_sample(s->logctx, "affected/current_dm_metadata_id " + "mismatch? %u != %u\n", affected_dm_id, current_dm_id); + ff_dovi_ctx_unref(s); + return AVERROR_PATCHWELCOME; } - if (!s->vdr[current_dm_id]) { - av_log(s->logctx, AV_LOG_ERROR, "Unknown previous RPU DM ID: %u\n", - current_dm_id); - goto fail; + if (!s->dm) { + s->dm = ff_refstruct_allocz(sizeof(AVDOVIColorMetadata)); + if (!s->dm) { + ff_dovi_ctx_unref(s); + return AVERROR(ENOMEM); + } } - /* Update current pointer based on current_dm_id */ - vdr = s->vdr[current_dm_id]; - s->color = &vdr->color; - - /* Update values of affected_dm_id */ - vdr = s->vdr[affected_dm_id]; - color = &vdr->color; + s->color = color = s->dm; color->dm_metadata_id = affected_dm_id; color->scene_refresh_flag = get_ue_golomb_31(gb); for (int i = 0; i < 9; i++) diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c index 242cd76c58..59e8ed6e3e 100644 --- a/libavcodec/dovi_rpuenc.c +++ b/libavcodec/dovi_rpuenc.c @@ -479,12 +479,6 @@ int ff_dovi_rpu_generate(DOVIContext *s, const AVDOVIMetadata *metadata, return AVERROR(ENOMEM); } - if (!s->vdr[color->dm_metadata_id]) { - s->vdr[color->dm_metadata_id] = ff_refstruct_allocz(sizeof(DOVIVdr)); - if (!s->vdr[color->dm_metadata_id]) - return AVERROR(ENOMEM); - } - num_ext_blocks_v1 = num_ext_blocks_v2 = 0; for (int i = 0; i < metadata->num_ext_blocks; i++) { const AVDOVIDmData *dm = av_dovi_get_ext(metadata, i); @@ -515,6 +509,12 @@ int ff_dovi_rpu_generate(DOVIContext *s, const AVDOVIMetadata *metadata, vdr_dm_metadata_present = memcmp(color, &ff_dovi_color_default, sizeof(*color)); use_prev_vdr_rpu = !memcmp(&s->vdr[vdr_rpu_id]->mapping, mapping, sizeof(*mapping)); + if (vdr_dm_metadata_present && !s->dm) { + s->dm = ff_refstruct_allocz(sizeof(AVDOVIColorMetadata)); + if (!s->dm) + return AVERROR(ENOMEM); + } + buffer_size = 12 /* vdr seq info */ + 5 /* CRC32 + terminator */; buffer_size += num_ext_blocks_v1 * 13; buffer_size += num_ext_blocks_v2 * 28; @@ -655,8 +655,8 @@ int ff_dovi_rpu_generate(DOVIContext *s, const AVDOVIMetadata *metadata, put_bits(pb, 12, color->source_max_pq); put_bits(pb, 10, color->source_diagonal); - memcpy(&s->vdr[color->dm_metadata_id]->color, color, sizeof(*color)); - s->color = &s->vdr[color->dm_metadata_id]->color; + memcpy(s->dm, color, sizeof(*color)); + s->color = s->dm; } set_ue_golomb(pb, num_ext_blocks_v1); -- 2.45.1 _______________________________________________ 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".