| Message ID | NXmySQC--3-9@lynne.ee |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,RFC,1/5] hwcontext: add a new AVHWFramesContext.opaque field | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
Lynne: > + dst->opaque = av_buffer_ref(src->opaque); > + if (!dst->internal->source_frames) { > + ret = AVERROR(ENOMEM); > + goto fail; > + } Don't you want to check for !dst->opaque? - Andreas
Jun 13, 2023, 06:43 by andreas.rheinhardt@outlook.com: > Lynne: > >> + dst->opaque = av_buffer_ref(src->opaque); >> + if (!dst->internal->source_frames) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> > > Don't you want to check for !dst->opaque? > > - Andreas > Yes, copy and paste error. Fixed.
Lynne: > + dst->opaque = av_buffer_ref(src->opaque); > + if (!dst->opaque) { > + ret = AVERROR(ENOMEM); > + goto fail; > + } According to the doxy, opaque can be left unset (and will be given that it is a new field), yet av_buffer_ref(NULL) will crash. We have av_buffer_replace() for something like that. How has this even been tested? - Andreas
Jun 13, 2023, 18:56 by andreas.rheinhardt@outlook.com: > Lynne: > >> + dst->opaque = av_buffer_ref(src->opaque); >> + if (!dst->opaque) { >> + ret = AVERROR(ENOMEM); >> + goto fail; >> + } >> > > According to the doxy, opaque can be left unset (and will be given that > it is a new field), yet av_buffer_ref(NULL) will crash. We have > av_buffer_replace() for something like that. > Fixed.
Jun 13, 2023, 06:20 by dev@lynne.ee: > This fixes three memory leaks from ff_vk_load_props(). > Does not depend on any other patches. > > Patch attached. > Pushed.
Jun 13, 2023, 06:21 by dev@lynne.ee: > Fixes a small memory leak. > This also prevents leaks on malloc/mutex init errors. > > Does not depend on any other patches. > > Patch attached. > Pushed.
Jun 13, 2023, 19:18 by dev@lynne.ee: > Jun 13, 2023, 18:56 by andreas.rheinhardt@outlook.com: > >> Lynne: >> >>> + dst->opaque = av_buffer_ref(src->opaque); >>> + if (!dst->opaque) { >>> + ret = AVERROR(ENOMEM); >>> + goto fail; >>> + } >>> >> >> According to the doxy, opaque can be left unset (and will be given that >> it is a new field), yet av_buffer_ref(NULL) will crash. We have >> av_buffer_replace() for something like that. >> > > Fixed. > Ping.
Jun 16, 2023, 13:02 by dev@lynne.ee: > Jun 13, 2023, 19:18 by dev@lynne.ee: > >> Jun 13, 2023, 18:56 by andreas.rheinhardt@outlook.com: >> >>> Lynne: >>> >>>> + dst->opaque = av_buffer_ref(src->opaque); >>>> + if (!dst->opaque) { >>>> + ret = AVERROR(ENOMEM); >>>> + goto fail; >>>> + } >>>> >>> >>> According to the doxy, opaque can be left unset (and will be given that >>> it is a new field), yet av_buffer_ref(NULL) will crash. We have >>> av_buffer_replace() for something like that. >>> >> >> Fixed. >> > > Ping. > Consider this out of RFC state.
From a9bdbbb64acfcb0540727895b7be4027ab9955f9 Mon Sep 17 00:00:00 2001 From: Lynne <dev@lynne.ee> Date: Tue, 13 Jun 2023 06:10:50 +0200 Subject: [PATCH 1/5] hwcontext: add a new AVHWFramesContext.opaque field This is a public field, settable before frames context initialization, and propagated to any derived contexts. API users can use it to store state and determine which context is one of theirs. This also allows decoders which create their own contexts to store some state which would be freed only at context destruction. --- libavutil/hwcontext.c | 6 ++++++ libavutil/hwcontext.h | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/libavutil/hwcontext.c b/libavutil/hwcontext.c index 3396598269..92a8cc907a 100644 --- a/libavutil/hwcontext.c +++ b/libavutil/hwcontext.c @@ -238,6 +238,7 @@ static void hwframe_ctx_free(void *opaque, uint8_t *data) av_buffer_unref(&ctx->internal->source_frames); av_buffer_unref(&ctx->device_ref); + av_buffer_unref(&ctx->opaque); av_freep(&ctx->hwctx); av_freep(&ctx->internal->priv); @@ -913,6 +914,11 @@ int av_hwframe_ctx_create_derived(AVBufferRef **derived_frame_ctx, dst->sw_format = src->sw_format; dst->width = src->width; dst->height = src->height; + dst->opaque = av_buffer_ref(src->opaque); + if (!dst->internal->source_frames) { + ret = AVERROR(ENOMEM); + goto fail; + } dst->internal->source_frames = av_buffer_ref(source_frame_ctx); if (!dst->internal->source_frames) { diff --git a/libavutil/hwcontext.h b/libavutil/hwcontext.h index 7ff08c8608..7655bee33f 100644 --- a/libavutil/hwcontext.h +++ b/libavutil/hwcontext.h @@ -227,6 +227,15 @@ typedef struct AVHWFramesContext { * Must be set by the user before calling av_hwframe_ctx_init(). */ int width, height; + + /** + * Opaque data. Can be set before calling av_hwframe_ctx_init(). + * MUST NOT be set afterwards. Will be unref'd along with the + * main context at closure. + * + * Will be propagated to any derived contexts. + */ + AVBufferRef *opaque; } AVHWFramesContext; /** -- 2.40.1