[FFmpeg-devel] [PATCH v0 09/14] avcodec: add private side data set to AVCodecInternal

[Anton Khirnov]

Quoting Jan Ekström (2023-03-24 18:34:28)
> On Fri, Mar 24, 2023 at 12:51 PM Anton Khirnov <anton at khirnov.net> wrote:
> >
> > Quoting Jan Ekström (2023-03-21 00:34:03)
> > > This allows configuring an encoder by using AVFrameSideData.
> > > ---
> > >  libavcodec/avcodec.c  | 1 +
> > >  libavcodec/internal.h | 7 +++++++
> > >  libavcodec/options.c  | 5 +++++
> > >  3 files changed, 13 insertions(+)
> > >
> > > diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> > > index c110b19e08..3faabe77d1 100644
> > > --- a/libavcodec/avcodec.c
> > > +++ b/libavcodec/avcodec.c
> > > @@ -403,6 +403,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
> > >      avci->nb_draining_errors = 0;
> > >      av_frame_unref(avci->buffer_frame);
> > >      av_packet_unref(avci->buffer_pkt);
> > > +    av_side_data_set_wipe(&avci->side_data_set);
> > >
> > >      if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
> > >          ff_thread_flush(avctx);
> > > diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> > > index f21101752d..c658e97313 100644
> > > --- a/libavcodec/internal.h
> > > +++ b/libavcodec/internal.h
> > > @@ -168,6 +168,13 @@ typedef struct AVCodecInternal {
> > >       * a boolean to describe whether context is opened or not.
> > >       */
> > >      unsigned int ctx_opened;
> > > +
> > > +    /**
> > > +     * Set holding static side data, such as HDR10 CLL / MDCV structures.
> > > +     * - encoding: set by user
> > > +     * - decoding: unused
> > > +     */
> > > +    AVFrameSideDataSet side_data_set;
> >
> > Why put it here and not in the public struct? It seems way more natural
> > there.
> >
> 
> The general idea was that if you want to make people utilize helpers
> and not touch entries willy-nilly,

But do we? Why?

In this case I see no advantage to having a public function over having
two public fields. The function is strictly worse because it cannot be
extended, and enlarges the symbol table.

-- 
Anton Khirnov