[FFmpeg-devel] [PATCH 2/2] lavf: fix AVStream private fields marker

[wm4]

On Tue, 14 Feb 2017 13:50:16 -0300
James Almer <jamrial at gmail.com> wrote:

> On 2/13/2017 7:51 AM, wm4 wrote:
> > Public fields were added after the private fields (negating the entire
> > point of this). New private fields go into AVStreamInternal anyway.
> > 
> > The new marker was set by guessing which fields are supposed to be
> > private and wshich not. recommended_encoder_configuration is accessed by
> > ffserver_config.c directly, and is supposed to use the public API.
> > 
> > ffmpeg.c accesses AVStream.cur_dts, even though it's a private field,
> > but that seems to be an older error.
> > ---
> >  libavformat/avformat.h | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> > index 64180bca9e..4c1b18e002 100644
> > --- a/libavformat/avformat.h
> > +++ b/libavformat/avformat.h
> > @@ -1005,7 +1005,9 @@ typedef struct AVStream {
> >       * All fields below this line are not part of the public API. They
> >       * may not be used outside of libavformat and can be changed and
> >       * removed at will.
> > -     * New public fields should be added right above.
> > +     * Internal note: be aware that physically removing these fields
> > +     * will break ABI. Replace removed fields with dummy fields, and
> > +     * add new fields to AVStreamInternal.
> >       *****************************************************************
> >       */
> >  
> > @@ -1201,6 +1203,12 @@ typedef struct AVStream {
> >       */
> >      int inject_global_side_data;
> >  
> > +    /*****************************************************************
> > +     * All fields above this line are not part of the public API.
> > +     * Fields below are part of the public API and ABI again.
> > +     *****************************************************************
> > +     */
> > +
> >      /**
> >       * String containing paris of key and values describing recommended encoder configuration.
> >       * Paris are separated by ','.
> >   
> 
> This is incredibly confusing. Wouldn't it be a better idea to just wait
> until the next major bump and reorder the fields as needed?
> 
> And for that matter, all the private fields are supposed to be moved to
> AVStreamInternal as well at the next bump.

The only way to fix this is a major bump. Until then, we shouldn't
document public fields like codecpar as private, because that's even
more confusing.