| Message ID | 20171105123451.22556-1-michael@niedermayer.cc |
|---|---|
| State | New |
| Headers | show |
On 11/5/2017 9:34 AM, Michael Niedermayer wrote: > This gives libavcodec a field that it can freely and safely use. > Avoiding the need of wraping of a users opaque_ref field and its issues. Could this perhaps be in an opaque internal struct instead, much like AVCodecInternal and whatnot? As wm4 said in the relevant discussion, this approach is nonoptimal and *will* snowball into a mess of fields if other libav* libraries start requiring their own buffers in a frame. An internal field of an opaque struct being in a public header is much cleaner and easier to maintain than adding such specific fields that may at some point in the future need to be removed. No actual comments about the approach in question to solve the issue. Will leave that to someone more knowledgeable. But at least I'm glad something is being done about it. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavutil/frame.c | 8 +++++++- > libavutil/frame.h | 10 ++++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 982fbb5c81..6ddaef1e74 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > #endif > > av_buffer_unref(&dst->opaque_ref); > + av_buffer_unref(&dst->avcodec_private_ref); > if (src->opaque_ref) { > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > if (!dst->opaque_ref) > return AVERROR(ENOMEM); > } > - > + if (src->avcodec_private_ref) { > + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_private_ref); > + if (!dst->avcodec_private_ref) > + return AVERROR(ENOMEM); > + } > return 0; > } > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > av_buffer_unref(&frame->hw_frames_ctx); > > av_buffer_unref(&frame->opaque_ref); > + av_buffer_unref(&frame->avcodec_private_ref); > > get_frame_defaults(frame); > } > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 0c6aab1c02..73b7d949a9 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -563,6 +563,16 @@ typedef struct AVFrame { > /** > * @} > */ > + /** > + * AVBufferRef for free use by libavcodec. Code outside avcodec will never > + * check or change the contents of the buffer ref. FFmpeg calls > + * av_buffer_unref() on it when the frame is unreferenced. > + * av_frame_copy_props() calls create a new reference with av_buffer_ref() > + * for the target frame's avcodec_private_ref field. > + * > + * avcodec should never assign mutually incompatible types to this field. > + */ > + AVBufferRef *avcodec_private_ref; > } AVFrame; > > #if FF_API_FRAME_GET_SET >
On Sun, Nov 5, 2017 at 1:34 PM, Michael Niedermayer <michael@niedermayer.cc> wrote: > This gives libavcodec a field that it can freely and safely use. > Avoiding the need of wraping of a users opaque_ref field and its issues. > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavutil/frame.c | 8 +++++++- > libavutil/frame.h | 10 ++++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 982fbb5c81..6ddaef1e74 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > #endif > > av_buffer_unref(&dst->opaque_ref); > + av_buffer_unref(&dst->avcodec_private_ref); > if (src->opaque_ref) { > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > if (!dst->opaque_ref) > return AVERROR(ENOMEM); > } > - > + if (src->avcodec_private_ref) { > + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_private_ref); > + if (!dst->avcodec_private_ref) > + return AVERROR(ENOMEM); > + } > return 0; > } > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > av_buffer_unref(&frame->hw_frames_ctx); > > av_buffer_unref(&frame->opaque_ref); > + av_buffer_unref(&frame->avcodec_private_ref); > > get_frame_defaults(frame); > } > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 0c6aab1c02..73b7d949a9 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -563,6 +563,16 @@ typedef struct AVFrame { > /** > * @} > */ > + /** > + * AVBufferRef for free use by libavcodec. Code outside avcodec will never > + * check or change the contents of the buffer ref. FFmpeg calls > + * av_buffer_unref() on it when the frame is unreferenced. > + * av_frame_copy_props() calls create a new reference with av_buffer_ref() > + * for the target frame's avcodec_private_ref field. > + * > + * avcodec should never assign mutually incompatible types to this field. > + */ > + AVBufferRef *avcodec_private_ref; > } AVFrame; > > #if FF_API_FRAME_GET_SET I would prefer if this field would not be library-specific, but perhaps just "private_ref" which is not allowed to be touched by users, and documented to only be valid while within one library - ie. if you pass a frame from avcodec to avfilter, avfilter could take over the field (and just free any info, if its still in there). This would avoid any chances of adding a multitude of fields later, and a single AVFrame instance is not going to be used in multiple libraries at the same time anyway (the contents might, but not the actual AVFrame struct) - Hendrik
On Sun, Nov 05, 2017 at 10:35:06AM -0300, James Almer wrote: > On 11/5/2017 9:34 AM, Michael Niedermayer wrote: > > This gives libavcodec a field that it can freely and safely use. > > Avoiding the need of wraping of a users opaque_ref field and its issues. > > Could this perhaps be in an opaque internal struct instead, much like > AVCodecInternal and whatnot? We could do a AVFrameInternal but that would require some differenecs to AVCodecInternal. The AVBufferRef from the patch has its own reference counting and destruction callback. (which avcodec can use for cleaning it up) a straight AVFrameInternal (in AVCodecInternal style) would not have that, we could place the AVFrameInternal inside a AVBufferRef or provide seperate API / fields to make cleanup from libavcodec possible. > As wm4 said in the relevant discussion, > this approach is nonoptimal and *will* snowball into a mess of fields if > other libav* libraries start requiring their own buffers in a frame. I remember and i think i didnt reply there but this is not really a plausible scenario. If we added a field per lib we would have realistically 2 fields one for libavcodec and one for libavfilter. Beyond that even with constructed use cases it seems to become hard (at least to me ATM) to see what else would be added. libswscale ? libavformat ? but what would they do with their fields ? But really i primarly want this to move forward, i have no real preferrance as long as we dont cram muliple opaques in a single opaque field with wraping or other. > An internal field of an opaque struct being in a public header is much > cleaner and easier to maintain than adding such specific fields that may > at some point in the future need to be removed. > > No actual comments about the approach in question to solve the issue. > Will leave that to someone more knowledgeable. But at least I'm glad > something is being done about it. > [...]
On Sun, Nov 05, 2017 at 02:52:50PM +0100, Hendrik Leppkes wrote: > On Sun, Nov 5, 2017 at 1:34 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > This gives libavcodec a field that it can freely and safely use. > > Avoiding the need of wraping of a users opaque_ref field and its issues. > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavutil/frame.c | 8 +++++++- > > libavutil/frame.h | 10 ++++++++++ > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/libavutil/frame.c b/libavutil/frame.c > > index 982fbb5c81..6ddaef1e74 100644 > > --- a/libavutil/frame.c > > +++ b/libavutil/frame.c > > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > > #endif > > > > av_buffer_unref(&dst->opaque_ref); > > + av_buffer_unref(&dst->avcodec_private_ref); > > if (src->opaque_ref) { > > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > > if (!dst->opaque_ref) > > return AVERROR(ENOMEM); > > } > > - > > + if (src->avcodec_private_ref) { > > + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_private_ref); > > + if (!dst->avcodec_private_ref) > > + return AVERROR(ENOMEM); > > + } > > return 0; > > } > > > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > > av_buffer_unref(&frame->hw_frames_ctx); > > > > av_buffer_unref(&frame->opaque_ref); > > + av_buffer_unref(&frame->avcodec_private_ref); > > > > get_frame_defaults(frame); > > } > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index 0c6aab1c02..73b7d949a9 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -563,6 +563,16 @@ typedef struct AVFrame { > > /** > > * @} > > */ > > + /** > > + * AVBufferRef for free use by libavcodec. Code outside avcodec will never > > + * check or change the contents of the buffer ref. FFmpeg calls > > + * av_buffer_unref() on it when the frame is unreferenced. > > + * av_frame_copy_props() calls create a new reference with av_buffer_ref() > > + * for the target frame's avcodec_private_ref field. > > + * > > + * avcodec should never assign mutually incompatible types to this field. > > + */ > > + AVBufferRef *avcodec_private_ref; > > } AVFrame; > > > > #if FF_API_FRAME_GET_SET > > I would prefer if this field would not be library-specific, but > perhaps just "private_ref" which is not allowed to be touched by > users, and documented to only be valid while within one library - ie. > if you pass a frame from avcodec to avfilter, avfilter could take over > the field (and just free any info, if its still in there). > This would avoid any chances of adding a multitude of fields later, > and a single AVFrame instance is not going to be used in multiple > libraries at the same time anyway (the contents might, but not the > actual AVFrame struct) that should be easy to implement ... a disadvantage is the slightly higher chance of mixing up types if some codepath doesnt cleanup the field question is what do most prefer ? avcodec_private_ref ? (that is one for each of the 2 libs) private_ref ? avframe_internal_ref ? (that is a private struct defined in avutil similar to AVCodecInternal) [...]
On 5 November 2017 at 13:52, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > On Sun, Nov 5, 2017 at 1:34 PM, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > This gives libavcodec a field that it can freely and safely use. > > Avoiding the need of wraping of a users opaque_ref field and its issues. > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavutil/frame.c | 8 +++++++- > > libavutil/frame.h | 10 ++++++++++ > > 2 files changed, 17 insertions(+), 1 deletion(-) > > > > diff --git a/libavutil/frame.c b/libavutil/frame.c > > index 982fbb5c81..6ddaef1e74 100644 > > --- a/libavutil/frame.c > > +++ b/libavutil/frame.c > > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS > > #endif > > > > av_buffer_unref(&dst->opaque_ref); > > + av_buffer_unref(&dst->avcodec_private_ref); > > if (src->opaque_ref) { > > dst->opaque_ref = av_buffer_ref(src->opaque_ref); > > if (!dst->opaque_ref) > > return AVERROR(ENOMEM); > > } > > - > > + if (src->avcodec_private_ref) { > > + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_ > private_ref); > > + if (!dst->avcodec_private_ref) > > + return AVERROR(ENOMEM); > > + } > > return 0; > > } > > > > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) > > av_buffer_unref(&frame->hw_frames_ctx); > > > > av_buffer_unref(&frame->opaque_ref); > > + av_buffer_unref(&frame->avcodec_private_ref); > > > > get_frame_defaults(frame); > > } > > diff --git a/libavutil/frame.h b/libavutil/frame.h > > index 0c6aab1c02..73b7d949a9 100644 > > --- a/libavutil/frame.h > > +++ b/libavutil/frame.h > > @@ -563,6 +563,16 @@ typedef struct AVFrame { > > /** > > * @} > > */ > > + /** > > + * AVBufferRef for free use by libavcodec. Code outside avcodec > will never > > + * check or change the contents of the buffer ref. FFmpeg calls > > + * av_buffer_unref() on it when the frame is unreferenced. > > + * av_frame_copy_props() calls create a new reference with > av_buffer_ref() > > + * for the target frame's avcodec_private_ref field. > > + * > > + * avcodec should never assign mutually incompatible types to this > field. > > + */ > > + AVBufferRef *avcodec_private_ref; > > } AVFrame; > > > > #if FF_API_FRAME_GET_SET > > I would prefer if this field would not be library-specific, but > perhaps just "private_ref" which is not allowed to be touched by > users, and documented to only be valid while within one library - ie. > if you pass a frame from avcodec to avfilter, avfilter could take over > the field (and just free any info, if its still in there). > This would avoid any chances of adding a multitude of fields later, > and a single AVFrame instance is not going to be used in multiple > libraries at the same time anyway (the contents might, but not the > actual AVFrame struct) > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > No, this would be horrible, lavfi should be able to use what's in the avbufferref without reinitializing anything that it should contain. lavu should handle anything that has to be initialized in the struct the avbufferref backs so that all internal libraries can use it.
On Sun, Nov 5, 2017 at 7:08 PM, Rostislav Pehlivanov <atomnuker@gmail.com> wrote: > On 5 November 2017 at 13:52, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > >> On Sun, Nov 5, 2017 at 1:34 PM, Michael Niedermayer >> <michael@niedermayer.cc> wrote: >> > This gives libavcodec a field that it can freely and safely use. >> > Avoiding the need of wraping of a users opaque_ref field and its issues. >> > >> > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >> > --- >> > libavutil/frame.c | 8 +++++++- >> > libavutil/frame.h | 10 ++++++++++ >> > 2 files changed, 17 insertions(+), 1 deletion(-) >> > >> > diff --git a/libavutil/frame.c b/libavutil/frame.c >> > index 982fbb5c81..6ddaef1e74 100644 >> > --- a/libavutil/frame.c >> > +++ b/libavutil/frame.c >> > @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS >> > #endif >> > >> > av_buffer_unref(&dst->opaque_ref); >> > + av_buffer_unref(&dst->avcodec_private_ref); >> > if (src->opaque_ref) { >> > dst->opaque_ref = av_buffer_ref(src->opaque_ref); >> > if (!dst->opaque_ref) >> > return AVERROR(ENOMEM); >> > } >> > - >> > + if (src->avcodec_private_ref) { >> > + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_ >> private_ref); >> > + if (!dst->avcodec_private_ref) >> > + return AVERROR(ENOMEM); >> > + } >> > return 0; >> > } >> > >> > @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) >> > av_buffer_unref(&frame->hw_frames_ctx); >> > >> > av_buffer_unref(&frame->opaque_ref); >> > + av_buffer_unref(&frame->avcodec_private_ref); >> > >> > get_frame_defaults(frame); >> > } >> > diff --git a/libavutil/frame.h b/libavutil/frame.h >> > index 0c6aab1c02..73b7d949a9 100644 >> > --- a/libavutil/frame.h >> > +++ b/libavutil/frame.h >> > @@ -563,6 +563,16 @@ typedef struct AVFrame { >> > /** >> > * @} >> > */ >> > + /** >> > + * AVBufferRef for free use by libavcodec. Code outside avcodec >> will never >> > + * check or change the contents of the buffer ref. FFmpeg calls >> > + * av_buffer_unref() on it when the frame is unreferenced. >> > + * av_frame_copy_props() calls create a new reference with >> av_buffer_ref() >> > + * for the target frame's avcodec_private_ref field. >> > + * >> > + * avcodec should never assign mutually incompatible types to this >> field. >> > + */ >> > + AVBufferRef *avcodec_private_ref; >> > } AVFrame; >> > >> > #if FF_API_FRAME_GET_SET >> >> I would prefer if this field would not be library-specific, but >> perhaps just "private_ref" which is not allowed to be touched by >> users, and documented to only be valid while within one library - ie. >> if you pass a frame from avcodec to avfilter, avfilter could take over >> the field (and just free any info, if its still in there). >> This would avoid any chances of adding a multitude of fields later, >> and a single AVFrame instance is not going to be used in multiple >> libraries at the same time anyway (the contents might, but not the >> actual AVFrame struct) >> >> - Hendrik >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > > No, this would be horrible, lavfi should be able to use what's in the > avbufferref without reinitializing anything that it should contain. lavu > should handle anything that has to be initialized in the struct the > avbufferref backs so that all internal libraries can use it. The entire point of this field is to contain opaque private data that no other library ever has any business of even knowing exists. If additional data needs to be shared between libraries, it should be a properly defined public shared structure. - Hendrik
On 11/5/2017 12:25 PM, Michael Niedermayer wrote: > On Sun, Nov 05, 2017 at 02:52:50PM +0100, Hendrik Leppkes wrote: >> On Sun, Nov 5, 2017 at 1:34 PM, Michael Niedermayer >> <michael@niedermayer.cc> wrote: >>> This gives libavcodec a field that it can freely and safely use. >>> Avoiding the need of wraping of a users opaque_ref field and its issues. >>> >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavutil/frame.c | 8 +++++++- >>> libavutil/frame.h | 10 ++++++++++ >>> 2 files changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavutil/frame.c b/libavutil/frame.c >>> index 982fbb5c81..6ddaef1e74 100644 >>> --- a/libavutil/frame.c >>> +++ b/libavutil/frame.c >>> @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS >>> #endif >>> >>> av_buffer_unref(&dst->opaque_ref); >>> + av_buffer_unref(&dst->avcodec_private_ref); >>> if (src->opaque_ref) { >>> dst->opaque_ref = av_buffer_ref(src->opaque_ref); >>> if (!dst->opaque_ref) >>> return AVERROR(ENOMEM); >>> } >>> - >>> + if (src->avcodec_private_ref) { >>> + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_private_ref); >>> + if (!dst->avcodec_private_ref) >>> + return AVERROR(ENOMEM); >>> + } >>> return 0; >>> } >>> >>> @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) >>> av_buffer_unref(&frame->hw_frames_ctx); >>> >>> av_buffer_unref(&frame->opaque_ref); >>> + av_buffer_unref(&frame->avcodec_private_ref); >>> >>> get_frame_defaults(frame); >>> } >>> diff --git a/libavutil/frame.h b/libavutil/frame.h >>> index 0c6aab1c02..73b7d949a9 100644 >>> --- a/libavutil/frame.h >>> +++ b/libavutil/frame.h >>> @@ -563,6 +563,16 @@ typedef struct AVFrame { >>> /** >>> * @} >>> */ >>> + /** >>> + * AVBufferRef for free use by libavcodec. Code outside avcodec will never >>> + * check or change the contents of the buffer ref. FFmpeg calls >>> + * av_buffer_unref() on it when the frame is unreferenced. >>> + * av_frame_copy_props() calls create a new reference with av_buffer_ref() >>> + * for the target frame's avcodec_private_ref field. >>> + * >>> + * avcodec should never assign mutually incompatible types to this field. >>> + */ >>> + AVBufferRef *avcodec_private_ref; >>> } AVFrame; >>> >>> #if FF_API_FRAME_GET_SET >> >> I would prefer if this field would not be library-specific, but >> perhaps just "private_ref" which is not allowed to be touched by >> users, and documented to only be valid while within one library - ie. >> if you pass a frame from avcodec to avfilter, avfilter could take over >> the field (and just free any info, if its still in there). >> This would avoid any chances of adding a multitude of fields later, >> and a single AVFrame instance is not going to be used in multiple >> libraries at the same time anyway (the contents might, but not the >> actual AVFrame struct) > > that should be easy to implement ... > > a disadvantage is the slightly higher chance of mixing up types if > some codepath doesnt cleanup the field > > question is what do most prefer ? > avcodec_private_ref ? (that is one for each of the 2 libs) > private_ref ? > avframe_internal_ref ? (that is a private struct defined in avutil similar to AVCodecInternal) Discard my suggestion. Being inside an internal opaque struct would require avpriv_ functions to access from within avcodec, and as BtbN mentioned on IRC earlier today we should definitely avoid that. So take Hendrik's suggestion, unless somebody starts a vote to force wm4's original implementation instead.
Am 05.11.2017 um 14:35 schrieb James Almer: > On 11/5/2017 9:34 AM, Michael Niedermayer wrote: >> This gives libavcodec a field that it can freely and safely use. >> Avoiding the need of wraping of a users opaque_ref field and its issues. > > Could this perhaps be in an opaque internal struct instead, much like > AVCodecInternal and whatnot? As wm4 said in the relevant discussion, > this approach is nonoptimal and *will* snowball into a mess of fields if > other libav* libraries start requiring their own buffers in a frame. > An internal field of an opaque struct being in a public header is much > cleaner and easier to maintain than adding such specific fields that may > at some point in the future need to be removed. The problem here is that avcodec, due to nested decoders and the like, might potentially wrap this field multiple times internally, so it basically has to be something avcodec specific.
>> I would prefer if this field would not be library-specific, but >> perhaps just "private_ref" which is not allowed to be touched by >> users, and documented to only be valid while within one library - ie. >> if you pass a frame from avcodec to avfilter, avfilter could take over >> the field (and just free any info, if its still in there). >> This would avoid any chances of adding a multitude of fields later, >> and a single AVFrame instance is not going to be used in multiple >> libraries at the same time anyway (the contents might, but not the >> actual AVFrame struct) > > that should be easy to implement ... > > a disadvantage is the slightly higher chance of mixing up types if > some codepath doesnt cleanup the field > > question is what do most prefer ? > avcodec_private_ref ? (that is one for each of the 2 libs) > private_ref ? > avframe_internal_ref ? (that is a private struct defined in avutil similar to AVCodecInternal) I like private_ref. Following this approach also keeps the diff to libav small.
On 11/6/2017 9:19 AM, Timo Rothenpieler wrote: > Am 05.11.2017 um 14:35 schrieb James Almer: >> On 11/5/2017 9:34 AM, Michael Niedermayer wrote: >>> This gives libavcodec a field that it can freely and safely use. >>> Avoiding the need of wraping of a users opaque_ref field and its issues. >> >> Could this perhaps be in an opaque internal struct instead, much like >> AVCodecInternal and whatnot? As wm4 said in the relevant discussion, >> this approach is nonoptimal and *will* snowball into a mess of fields if >> other libav* libraries start requiring their own buffers in a frame. >> An internal field of an opaque struct being in a public header is much >> cleaner and easier to maintain than adding such specific fields that may >> at some point in the future need to be removed. > > The problem here is that avcodec, due to nested decoders and the like, > might potentially wrap this field multiple times internally, so it > basically has to be something avcodec specific. And an opaque internal struct would require avpriv_ symbols to be accessed from outside libavutil, so it's an ugly solution nonetheless. I already told Michael to discard the suggestion.
diff --git a/libavutil/frame.c b/libavutil/frame.c index 982fbb5c81..6ddaef1e74 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -383,12 +383,17 @@ FF_ENABLE_DEPRECATION_WARNINGS #endif av_buffer_unref(&dst->opaque_ref); + av_buffer_unref(&dst->avcodec_private_ref); if (src->opaque_ref) { dst->opaque_ref = av_buffer_ref(src->opaque_ref); if (!dst->opaque_ref) return AVERROR(ENOMEM); } - + if (src->avcodec_private_ref) { + dst->avcodec_private_ref = av_buffer_ref(src->avcodec_private_ref); + if (!dst->avcodec_private_ref) + return AVERROR(ENOMEM); + } return 0; } @@ -524,6 +529,7 @@ void av_frame_unref(AVFrame *frame) av_buffer_unref(&frame->hw_frames_ctx); av_buffer_unref(&frame->opaque_ref); + av_buffer_unref(&frame->avcodec_private_ref); get_frame_defaults(frame); } diff --git a/libavutil/frame.h b/libavutil/frame.h index 0c6aab1c02..73b7d949a9 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -563,6 +563,16 @@ typedef struct AVFrame { /** * @} */ + /** + * AVBufferRef for free use by libavcodec. Code outside avcodec will never + * check or change the contents of the buffer ref. FFmpeg calls + * av_buffer_unref() on it when the frame is unreferenced. + * av_frame_copy_props() calls create a new reference with av_buffer_ref() + * for the target frame's avcodec_private_ref field. + * + * avcodec should never assign mutually incompatible types to this field. + */ + AVBufferRef *avcodec_private_ref; } AVFrame; #if FF_API_FRAME_GET_SET
This gives libavcodec a field that it can freely and safely use. Avoiding the need of wraping of a users opaque_ref field and its issues. Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavutil/frame.c | 8 +++++++- libavutil/frame.h | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-)