Re: [FFmpeg-devel] [PATCH 3/8] avcodec/dovi_rpuenc: try to re-use existing vdr_rpu_id

[Niklas Haas <ffmpeg@haasn.xyz>]

On Tue, 18 Jun 2024 21:35:33 +0200 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> And only override it if we either have an exact match, or if we still
> have unused metadata slots (to avoid an overwrite).
> ---
>  libavcodec/dovi_rpuenc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/dovi_rpuenc.c b/libavcodec/dovi_rpuenc.c
> index 45fcd9a86c..0d49a128fd 100644
> --- a/libavcodec/dovi_rpuenc.c
> +++ b/libavcodec/dovi_rpuenc.c
> @@ -463,12 +463,12 @@ int ff_dovi_rpu_generate(DOVIContext *s, const AVDOVIMetadata *metadata,
>          return AVERROR_INVALIDDATA;
>      }
>  
> -    vdr_rpu_id = -1;
> +    vdr_rpu_id = mapping->vdr_rpu_id;
>      for (int i = 0; i <= DOVI_MAX_DM_ID; i++) {
>          if (s->vdr[i] && !memcmp(s->vdr[i], mapping, sizeof(*mapping))) {
>              vdr_rpu_id = i;
>              break;
> -        } else if (vdr_rpu_id < 0 && (!s->vdr[i] || i == DOVI_MAX_DM_ID)) {
> +        } else if (s->vdr[vdr_rpu_id] && !s->vdr[i]) {
>              vdr_rpu_id = i;
>          }
>      }
> -- 
> 2.45.1
> 

I just noticed this check is wrong either way, because the `vdr_rpu_id`
is also included as part of `mapping`. If we reshuffle VDR IDs, we also
need to change the contents of `mapping` when updating the vdrs, as well
as omitting it when doing the `memcmp`.

Effectively this loop currently does nothing, since the only field that
can succeed is in the case `i == mapping->vdr_rpu_id`.

Will fix for v2.
_______________________________________________
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".