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 at 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 at 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)
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
"You are 36 times more likely to die in a bathtub than at the hands of a
terrorist. Also, you are 2.5 times more likely to become a president and
2 times more likely to become an astronaut, than to die in a terrorist
attack." -- Thoughty2
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171105/2f373973/attachment.sig>