Re: [FFmpeg-devel] [PATCH v5 1/6] libavcodec: Add generic metadata injection using AV_PKT_DATA_METADATA_UPDATE

[Lynne <dev@lynne.ee>]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 3291 bytes --]

On 17/02/2025 17:19, Romain Beauxis wrote:
> libavcodec/decode.c: intercept `AV_PKT_DATA_METADATA_UPDATE` packet extra data,
> attach them to the next decoded frame.
> 
> The metadata needs to be cached because there is no guarantee that each packet
> will generate a decoded frame immediately.
> 
> `AV_PKT_DATA_METADATA_UPDATE` does not seem used in `libavcodec` at the moment
> so regression risks seem low.
> 
> This generic mechanism could be reused to support more in-band metadata update
> in the future.
> 
> ---
>   libavcodec/decode.c | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index cac7e620d2..96e2f0ce95 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -97,6 +97,8 @@ typedef struct DecodeContext {
>       int lcevc_frame;
>       int width;
>       int height;
> +
> +    AVDictionary *pending_metadata;
>   } DecodeContext;
>   
>   static DecodeContext *decode_ctx(AVCodecInternal *avci)
> @@ -729,6 +731,8 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>   {
>       AVCodecInternal *avci = avctx->internal;
>       DecodeContext     *dc = decode_ctx(avci);
> +    const uint8_t *side_metadata;
> +    size_t size;
>       int ret;
>   
>       if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
> @@ -746,6 +750,14 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
>           ret = av_packet_ref(avci->buffer_pkt, avpkt);
>           if (ret < 0)
>               return ret;
> +
> +        side_metadata = av_packet_get_side_data(avpkt, AV_PKT_DATA_METADATA_UPDATE, &size);
> +        if (side_metadata) {
> +            av_dict_free(&dc->pending_metadata);
> +            ret = av_packet_unpack_dictionary(side_metadata, size, &dc->pending_metadata);
> +            if (ret < 0)
> +                return ret;
> +        }
>       } else
>           dc->draining_started = 1;
>   
> @@ -815,6 +827,7 @@ fail:
>   int ff_decode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>   {
>       AVCodecInternal *avci = avctx->internal;
> +    DecodeContext     *dc = decode_ctx(avci);
>       int ret;
>   
>       if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
> @@ -887,6 +900,12 @@ int ff_decode_receive_frame(AVCodecContext *avctx, AVFrame *frame)
>           }
>       }
>   #endif
> +
> +    if (dc->pending_metadata) {
> +        av_dict_copy(&frame->metadata, dc->pending_metadata, AV_DICT_APPEND);
> +        av_dict_free(&dc->pending_metadata);
> +    }
> +
>       return 0;
>   fail:
>       av_frame_unref(frame);
> @@ -2314,4 +2333,5 @@ void ff_decode_internal_uninit(AVCodecContext *avctx)
>       DecodeContext *dc = decode_ctx(avci);
>   
>       av_refstruct_unref(&dc->lcevc);
> +    av_dict_free(&dc->pending_metadata);
>   }

btw, jamrial mentioned that AVSTREAM_EVENT_FLAG_METADATA_UPDATED exists.
I only see it used to inform API users that metadata has been updated, 
but you should set the flag.

Other than that, I'm fine with the patchset. You should resend it with 
the change, and if there are no comments, I'll merge it in a few days.

Thanks

[-- Attachment #1.1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 637 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 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".