[FFmpeg-devel] [PATCH 1/2] avcodec/decode: make ff_frame_new_side_data_from_buf behave like the function it's a wrapper for
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Fri Feb 7 14:17:49 EET 2025
James Almer:
> It's less confusing to have ff_frame_new_side_data_from_buf() be a drop in
> replacement for av_frame_side_data_add(), and the addition of the missing flags
> field will make it more versatile, as will be seen in the following commit.
>
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
> libavcodec/av1dec.c | 6 ++++--
> libavcodec/decode.c | 13 +++++--------
> libavcodec/decode.h | 11 +++++++----
> libavcodec/h2645_sei.c | 12 ++++++++----
> libavcodec/hevc/hevcdec.c | 7 +++++--
> libavcodec/hevc/refs.c | 6 ++++--
> libavcodec/libdav1d.c | 6 ++++--
> libavcodec/libjxldec.c | 6 ++++--
> libavcodec/mpeg12dec.c | 6 ++++--
> 9 files changed, 45 insertions(+), 28 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 7c54e36220..11b6100f9f 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -983,9 +983,11 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
> if (!ret)
> break;
>
> - ret = ff_frame_new_side_data_from_buf(avctx, frame, AV_FRAME_DATA_A53_CC, &buf);
> - if (ret < 0)
> + ret = ff_frame_new_side_data_from_buf(avctx, frame, AV_FRAME_DATA_A53_CC, &buf, 0);
> + if (ret < 0) {
> + av_buffer_unref(&buf);
> return ret;
> + }
>
> #if FF_API_CODEC_PROPS
> FF_DISABLE_DEPRECATION_WARNINGS
> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
> index cac7e620d2..794e84c1a2 100644
> --- a/libavcodec/decode.c
> +++ b/libavcodec/decode.c
> @@ -2118,29 +2118,26 @@ int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrame *frame,
> int ff_frame_new_side_data_from_buf_ext(const AVCodecContext *avctx,
> AVFrameSideData ***sd, int *nb_sd,
> enum AVFrameSideDataType type,
> - AVBufferRef **buf)
> + AVBufferRef **buf, int flags)
> {
> int ret = 0;
>
> if (side_data_pref(avctx, sd, nb_sd, type))
> - goto finish;
> + return 0;
>
> - if (!av_frame_side_data_add(sd, nb_sd, type, buf, 0))
> + if (!av_frame_side_data_add(sd, nb_sd, type, buf, flags))
> ret = AVERROR(ENOMEM);
>
> -finish:
> - av_buffer_unref(buf);
> -
> return ret;
> }
>
> int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx,
> AVFrame *frame, enum AVFrameSideDataType type,
> - AVBufferRef **buf)
> + AVBufferRef **buf, int flags)
> {
> return ff_frame_new_side_data_from_buf_ext(avctx,
> &frame->side_data, &frame->nb_side_data,
> - type, buf);
> + type, buf, flags);
> }
>
> int ff_decode_mastering_display_new_ext(const AVCodecContext *avctx,
> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
> index 2c3719a8d0..dc2802d782 100644
> --- a/libavcodec/decode.h
> +++ b/libavcodec/decode.h
> @@ -168,12 +168,15 @@ int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrame *frame,
> /**
> * Similar to `ff_frame_new_side_data`, but using an existing buffer ref.
> *
> - * *buf is ALWAYS consumed by this function and NULL written in its place, even
> - * on failure.
> + * @param buf Pointer to AVBufferRef to add to the array. On success,
> + * the function takes ownership of the AVBufferRef and *buf is
> + * set to NULL, unless AV_FRAME_SIDE_DATA_FLAG_NEW_REF is set
> + * in which case the ownership will remain with the caller.
> + * @param flags Some combination of AV_FRAME_SIDE_DATA_FLAG_* flags, or 0.
> */
> int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx,
> AVFrame *frame, enum AVFrameSideDataType type,
> - AVBufferRef **buf);
> + AVBufferRef **buf, int flags);
>
> /**
> * Same as `ff_frame_new_side_data_from_buf`, but taking a AVFrameSideData
> @@ -182,7 +185,7 @@ int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx,
> int ff_frame_new_side_data_from_buf_ext(const AVCodecContext *avctx,
> AVFrameSideData ***sd, int *nb_sd,
> enum AVFrameSideDataType type,
> - AVBufferRef **buf);
> + AVBufferRef **buf, int flags);
>
> struct AVMasteringDisplayMetadata;
> struct AVContentLightMetadata;
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index 2494daaf3c..d48a9b5f2c 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -610,10 +610,12 @@ static int h2645_sei_to_side_data(AVCodecContext *avctx, H2645SEI *sei,
> }
>
> ret = ff_frame_new_side_data_from_buf_ext(avctx, sd, nb_sd,
> - AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT, &buf);
> + AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT, &buf, 0);
>
> - if (ret < 0)
> + if (ret < 0) {
> + av_buffer_unref(&buf);
> return ret;
> + }
>
> dst_env->ambient_illuminance = av_make_q(env->ambient_illuminance, 10000);
> dst_env->ambient_light_x = av_make_q(env->ambient_light_x, 50000);
> @@ -830,9 +832,11 @@ FF_ENABLE_DEPRECATION_WARNINGS
>
> if (sei->lcevc.info) {
> HEVCSEILCEVC *lcevc = &sei->lcevc;
> - ret = ff_frame_new_side_data_from_buf(avctx, frame, AV_FRAME_DATA_LCEVC, &lcevc->info);
> - if (ret < 0)
> + ret = ff_frame_new_side_data_from_buf(avctx, frame, AV_FRAME_DATA_LCEVC, &lcevc->info, 0);
> + if (ret < 0) {
> + av_buffer_unref(&lcevc->info);
> return ret;
> + }
> }
>
> if (sei->film_grain_characteristics && sei->film_grain_characteristics->present) {
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index e9c045f7a1..cf4062c75c 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -3085,9 +3085,12 @@ static int set_side_data(HEVCContext *s)
> if (!info_ref)
> return AVERROR(ENOMEM);
>
> - ret = ff_frame_new_side_data_from_buf(s->avctx, out, AV_FRAME_DATA_DYNAMIC_HDR_PLUS, &info_ref);
> - if (ret < 0)
> + ret = ff_frame_new_side_data_from_buf(s->avctx, out, AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> + &info_ref, 0);
> + if (ret < 0) {
> + av_buffer_unref(&info_ref);
> return ret;
> + }
> }
>
> if (s->rpu_buf) {
> diff --git a/libavcodec/hevc/refs.c b/libavcodec/hevc/refs.c
> index dd7f7f95a8..04cecd9770 100644
> --- a/libavcodec/hevc/refs.c
> +++ b/libavcodec/hevc/refs.c
> @@ -97,9 +97,11 @@ static HEVCFrame *alloc_frame(HEVCContext *s, HEVCLayerContext *l)
> if (s->sei.common.lcevc.info) {
> HEVCSEILCEVC *lcevc = &s->sei.common.lcevc;
> ret = ff_frame_new_side_data_from_buf(s->avctx, frame->tf.f,
> - AV_FRAME_DATA_LCEVC, &lcevc->info);
> - if (ret < 0)
> + AV_FRAME_DATA_LCEVC, &lcevc->info, 0);
> + if (ret < 0) {
> + av_buffer_unref(&lcevc->info);
> goto fail;
> + }
> }
>
> // add view ID side data if it's nontrivial
> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index f4cbc927b5..d2c7342e55 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -533,9 +533,11 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
> if (!res)
> break;
>
> - res = ff_frame_new_side_data_from_buf(c, frame, AV_FRAME_DATA_A53_CC, &buf);
> - if (res < 0)
> + res = ff_frame_new_side_data_from_buf(c, frame, AV_FRAME_DATA_A53_CC, &buf, 0);
> + if (res < 0) {
> + av_buffer_unref(&buf);
> goto fail;
> + }
>
> #if FF_API_CODEC_PROPS
> FF_DISABLE_DEPRECATION_WARNINGS
> diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
> index 96c338d1b4..a9261f5016 100644
> --- a/libavcodec/libjxldec.c
> +++ b/libavcodec/libjxldec.c
> @@ -483,9 +483,11 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
> /* full image is one frame, even if animated */
> av_log(avctx, AV_LOG_DEBUG, "FULL_IMAGE event emitted\n");
> if (ctx->iccp) {
> - ret = ff_frame_new_side_data_from_buf(avctx, ctx->frame, AV_FRAME_DATA_ICC_PROFILE, &ctx->iccp);
> - if (ret < 0)
> + ret = ff_frame_new_side_data_from_buf(avctx, ctx->frame, AV_FRAME_DATA_ICC_PROFILE, &ctx->iccp, 0);
> + if (ret < 0) {
> + av_buffer_unref(&ctx->iccp);
> return ret;
> + }
> }
> if (ctx->basic_info.have_animation) {
> ctx->frame->pts = av_rescale_q(ctx->accumulated_pts, ctx->anim_timebase, avctx->pkt_timebase);
> diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> index 0002f016e9..b181852b6e 100644
> --- a/libavcodec/mpeg12dec.c
> +++ b/libavcodec/mpeg12dec.c
> @@ -1308,9 +1308,11 @@ static int mpeg_field_start(Mpeg1Context *s1, const uint8_t *buf, int buf_size)
> if (s1->a53_buf_ref) {
> ret = ff_frame_new_side_data_from_buf(
> s->avctx, s->cur_pic.ptr->f, AV_FRAME_DATA_A53_CC,
> - &s1->a53_buf_ref);
> - if (ret < 0)
> + &s1->a53_buf_ref, 0);
> + if (ret < 0) {
> + av_buffer_unref(&s1->a53_buf_ref);
This adds these cleanup av_buffer_unref() everywhere. That is not an
improvement.
> return ret;
> + }
> }
>
> if (s1->has_stereo3d) {
More information about the ffmpeg-devel
mailing list