[FFmpeg-devel] [PATCH 1/2] avcodec/decode: inject missing global side data to output frames

[FFmpeg-devel] [PATCH 1/2] avcodec/decode: inject missing global side data to output frames

[James Almer]

ff_decode_frame_props() injects global side data passed by the caller (Usually
coming from the container) but ignores the global side data the decoder
gathered from the bitstream itself.
This commit amends this.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/decode.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index cac7e620d2..c5a577f4f1 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1570,6 +1570,15 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
     if (ret < 0)
         return ret;
 
+    for (int i = 0; i < avctx->nb_decoded_side_data; i++) {
+        const AVFrameSideData *src = avctx->decoded_side_data[i];
+        if (av_frame_get_side_data(frame, src->type))
+            continue;
+        ret = av_frame_side_data_clone(&frame->side_data, &frame->nb_side_data, src, 0);
+        if (ret < 0)
+            return ret;
+    }
+
     if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
         const AVPacket *pkt = avctx->internal->last_pkt_props;
 
-- 
2.48.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".

[FFmpeg-devel] [PATCH 2/2] fftools/ffmpeg_dec: remove side data copy block

[James Almer]

It's no longer needed now that lavc handles this.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 fftools/ffmpeg_dec.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fftools/ffmpeg_dec.c b/fftools/ffmpeg_dec.c
index 01a6933bcc..62edbb6c24 100644
--- a/fftools/ffmpeg_dec.c
+++ b/fftools/ffmpeg_dec.c
@@ -788,15 +788,6 @@ static int packet_decode(DecoderPriv *dp, AVPacket *pkt, AVFrame *frame)
 
         frame->time_base = dec->pkt_timebase;
 
-        for (int i = 0; i < dec->nb_decoded_side_data; i++) {
-            const AVFrameSideData *src = dec->decoded_side_data[i];
-            if (av_frame_side_data_get(frame->side_data, frame->nb_side_data, src->type))
-                continue;
-            ret = av_frame_side_data_clone(&frame->side_data, &frame->nb_side_data, src, 0);
-            if (ret < 0)
-                return ret;
-        }
-
         if (dec->codec_type == AVMEDIA_TYPE_AUDIO) {
             dp->dec.samples_decoded += frame->nb_samples;
 
-- 
2.48.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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: inject missing global side data to output frames

[Andreas Rheinhardt]

James Almer:
> ff_decode_frame_props() injects global side data passed by the caller (Usually
> coming from the container) but ignores the global side data the decoder
> gathered from the bitstream itself.
> This commit amends this.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/decode.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index cac7e620d2..c5a577f4f1 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -1570,6 +1570,15 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
>      if (ret < 0)
>          return ret;
>  
> +    for (int i = 0; i < avctx->nb_decoded_side_data; i++) {
> +        const AVFrameSideData *src = avctx->decoded_side_data[i];
> +        if (av_frame_get_side_data(frame, src->type))
> +            continue;
> +        ret = av_frame_side_data_clone(&frame->side_data, &frame->nb_side_data, src, 0);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
>      if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
>          const AVPacket *pkt = avctx->internal->last_pkt_props;
>  

Why is this attached to every frame instead of just the first one?
If I read this correctly, it will be possible for there to be
decoded_side_data propagated to the output, then in-stream side-data of
the same type (not overridden) and in the next frame, the decoder adds
the decoded_side_data again, presuming that the side data from the
earlier frame was meant to be non-persistent.

- Andreas

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: inject missing global side data to output frames

[James Almer]

[-- Attachment #1.1.1: Type: text/plain, Size: 2239 bytes --]

On 2/26/2025 1:57 PM, Andreas Rheinhardt wrote:
> James Almer:
>> ff_decode_frame_props() injects global side data passed by the caller (Usually
>> coming from the container) but ignores the global side data the decoder
>> gathered from the bitstream itself.
>> This commit amends this.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/decode.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index cac7e620d2..c5a577f4f1 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -1570,6 +1570,15 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
>>       if (ret < 0)
>>           return ret;
>>   
>> +    for (int i = 0; i < avctx->nb_decoded_side_data; i++) {
>> +        const AVFrameSideData *src = avctx->decoded_side_data[i];
>> +        if (av_frame_get_side_data(frame, src->type))
>> +            continue;
>> +        ret = av_frame_side_data_clone(&frame->side_data, &frame->nb_side_data, src, 0);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>> +
>>       if (!(ffcodec(avctx->codec)->caps_internal & FF_CODEC_CAP_SETS_FRAME_PROPS)) {
>>           const AVPacket *pkt = avctx->internal->last_pkt_props;
>>   
> 
> Why is this attached to every frame instead of just the first one?

These entries are stream wide and apply to every frame (unless 
overridden by frame specific side data), same as the global entries that 
came from the container that are attached immediately above this chunk.

> If I read this correctly, it will be possible for there to be
> decoded_side_data propagated to the output, then in-stream side-data of
> the same type (not overridden) and in the next frame, the decoder adds
> the decoded_side_data again, presuming that the side data from the
> earlier frame was meant to be non-persistent.

If a side data that's coded in a frame is meant persist beyond the frame 
where it appeared, then the decoder is who should take care of that by 
attaching it to every frame that needs it before returning it, and not 
the generic code.
This is the case with for example mastering display metadata in h264/5.


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".