diff mbox series

[FFmpeg-devel,07/12] avcodec/hevcdec: check for DOVI configuration record in AVCodecContext side data

Message ID 20230828123617.57535-8-jamrial@gmail.com
State New
Headers show
Series AVCodecContext and AVCodecParameters side data | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 fail Make failed
andriy/make_x86 fail Make failed

Commit Message

James Almer Aug. 28, 2023, 12:34 p.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/hevcdec.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Vittorio Giovara Aug. 28, 2023, 1 p.m. UTC | #1 
On Mon, Aug 28, 2023 at 2:38 PM James Almer <jamrial@gmail.com> wrote:

> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/hevcdec.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
> index df40c91ba6..dabfe89d4a 100644
> --- a/libavcodec/hevcdec.c
> +++ b/libavcodec/hevcdec.c
> @@ -3337,8 +3337,15 @@ static int hevc_decode_frame(AVCodecContext *avctx,
> AVFrame *rframe,
>      }
>
>      sd = av_packet_get_side_data(avpkt, AV_PKT_DATA_DOVI_CONF, &sd_size);
> -    if (sd && sd_size > 0)
> +    if (sd && sd_size > 0) {
> +        int old = s->dovi_ctx.dv_profile;
> +
>          ff_dovi_update_cfg(&s->dovi_ctx,
> (AVDOVIDecoderConfigurationRecord *) sd);
> +        if (old)
> +            av_log(avctx, AV_LOG_DEBUG,
> +                   "New DOVI configuration record from input packet
> (profile %d -> %u).\n",
> +                   old, s->dovi_ctx.dv_profile);
> +    }
>

In general isn't the bitstream side data more important than the container
level one?
Unless specified with a flag, I'd expect that the bistream information is
preserved over the other ones since it's harder to misconfure, and (iirc)
it's what we do in many other places (for example on color properties
config).
James Almer Aug. 28, 2023, 1:25 p.m. UTC | #2 
On 8/28/2023 10:00 AM, Vittorio Giovara wrote:
> On Mon, Aug 28, 2023 at 2:38 PM James Almer <jamrial@gmail.com> wrote:
> 
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/hevcdec.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
>> index df40c91ba6..dabfe89d4a 100644
>> --- a/libavcodec/hevcdec.c
>> +++ b/libavcodec/hevcdec.c
>> @@ -3337,8 +3337,15 @@ static int hevc_decode_frame(AVCodecContext *avctx,
>> AVFrame *rframe,
>>       }
>>
>>       sd = av_packet_get_side_data(avpkt, AV_PKT_DATA_DOVI_CONF, &sd_size);
>> -    if (sd && sd_size > 0)
>> +    if (sd && sd_size > 0) {
>> +        int old = s->dovi_ctx.dv_profile;
>> +
>>           ff_dovi_update_cfg(&s->dovi_ctx,
>> (AVDOVIDecoderConfigurationRecord *) sd);
>> +        if (old)
>> +            av_log(avctx, AV_LOG_DEBUG,
>> +                   "New DOVI configuration record from input packet
>> (profile %d -> %u).\n",
>> +                   old, s->dovi_ctx.dv_profile);
>> +    }
>>
> 
> In general isn't the bitstream side data more important than the container
> level one?
> Unless specified with a flag, I'd expect that the bistream information is
> preserved over the other ones since it's harder to misconfure, and (iirc)
> it's what we do in many other places (for example on color properties
> config).

This is the DOVI conf side data, which is read from the container 
header. The RPUs are still read exclusively from the bitstream.

Before this set it was being propagated by lavf as packet side data. 
After this set, lavf makes it available in avctx's side data. My 
original intention was to stop looking at packet side data here 
altogether, but Andreas argued that lavc users other than lavf may still 
propagate it that way, so i left it in place.
diff mbox series

Patch

diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index df40c91ba6..dabfe89d4a 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3337,8 +3337,15 @@  static int hevc_decode_frame(AVCodecContext *avctx, AVFrame *rframe,
     }
 
     sd = av_packet_get_side_data(avpkt, AV_PKT_DATA_DOVI_CONF, &sd_size);
-    if (sd && sd_size > 0)
+    if (sd && sd_size > 0) {
+        int old = s->dovi_ctx.dv_profile;
+
         ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd);
+        if (old)
+            av_log(avctx, AV_LOG_DEBUG,
+                   "New DOVI configuration record from input packet (profile %d -> %u).\n",
+                   old, s->dovi_ctx.dv_profile);
+    }
 
     s->ref = NULL;
     ret    = decode_nal_units(s, avpkt->data, avpkt->size);
@@ -3641,12 +3648,18 @@  static av_cold int hevc_decode_init(AVCodecContext *avctx)
     atomic_init(&s->wpp_err, 0);
 
     if (!avctx->internal->is_copy) {
+        AVPacketSideData *sd;
+
         if (avctx->extradata_size > 0 && avctx->extradata) {
             ret = hevc_decode_extradata(s, avctx->extradata, avctx->extradata_size, 1);
             if (ret < 0) {
                 return ret;
             }
         }
+
+        sd = av_packet_side_data_set_get(&avctx->packet_side_data, AV_PKT_DATA_DOVI_CONF);
+        if (sd && sd->size > 0)
+            ff_dovi_update_cfg(&s->dovi_ctx, (AVDOVIDecoderConfigurationRecord *) sd->data);
     }
 
     return 0;