[FFmpeg-cvslog] avcodec/dovi_rpu: fix dm_metadata_id handling
Niklas Haas
git at videolan.org
Tue Jun 18 22:18:52 EEST 2024
ffmpeg | branch: master | Niklas Haas <git at haasn.dev> | Sun Jun 9 15:04:45 2024 +0200| [7ee64631f625372a5796de46b1cda4c9a50d4b74] | committer: Niklas Haas
avcodec/dovi_rpu: fix dm_metadata_id handling
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.
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=7ee64631f625372a5796de46b1cda4c9a50d4b74
---
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 c577b521fb..65a062c2ab 100644
--- a/libavcodec/dovi_rpudec.c
+++ b/libavcodec/dovi_rpudec.c
@@ -580,25 +580,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 5d4945c345..3fd9046453 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;
} else {
s->color = &ff_dovi_color_default;
}
More information about the ffmpeg-cvslog
mailing list