[FFmpeg-devel] [PATCH] avutil/frame: Add avcodec_private_ref to AVFrame

[Hendrik Leppkes]

On Sun, Nov 5, 2017 at 7:08 PM, Rostislav Pehlivanov
<atomnuker at gmail.com> wrote:
> On 5 November 2017 at 13:52, Hendrik Leppkes <h.leppkes at gmail.com> 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)
>>
>> - Hendrik
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at 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