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

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(-)

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.

On Tue, 14 Feb 2017 13:50:16 -0300
James Almer <jamrial@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.