From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/decode: make ff_frame_new_side_data_from_buf behave like the function it's a wrapper for
Date: Fri, 7 Feb 2025 13:17:49 +0100
Message-ID: <AS8P250MB0744F0C8B1B461D68A8A1FA78FF12@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <20250204211256.10228-1-jamrial@gmail.com>
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@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) {
_______________________________________________
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".
next prev parent reply other threads:[~2025-02-07 12:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-04 21:12 James Almer
2025-02-04 21:12 ` [FFmpeg-devel] [PATCH 2/2] avcodec/hevc/hevcdec: simplify creating a new reference to HDR10 side data James Almer
2025-02-07 12:17 ` Andreas Rheinhardt [this message]
2025-02-07 16:22 ` [FFmpeg-devel] [PATCH 1/2] avcodec/decode: make ff_frame_new_side_data_from_buf behave like the function it's a wrapper for James Almer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AS8P250MB0744F0C8B1B461D68A8A1FA78FF12@AS8P250MB0744.EURP250.PROD.OUTLOOK.COM \
--to=andreas.rheinhardt@outlook.com \
--cc=ffmpeg-devel@ffmpeg.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git