[FFmpeg-devel] [PATCH 1/5] avformat: Adding accessors for externally used AVStream fields which are after the public field list

Hendrik Leppkes h.leppkes at gmail.com
Wed Jun 29 23:09:15 CEST 2016


On Wed, Jun 29, 2016 at 10:52 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Wed, Jun 29, 2016 at 10:28:27PM +0200, Hendrik Leppkes wrote:
>> On Wed, Jun 29, 2016 at 10:18 PM, Michael Niedermayer
>> <michael at niedermayer.cc> wrote:
>> > On Wed, Jun 29, 2016 at 09:41:50PM +0200, Clément Bœsch wrote:
>> >> On Wed, Jun 29, 2016 at 09:36:39PM +0200, Michael Niedermayer wrote:
>> >> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
>> >> > ---
>> >> >  libavformat/avformat.h |    4 ++++
>> >> >  libavformat/utils.c    |   21 +++++++++++++++++++++
>> >> >  2 files changed, 25 insertions(+)
>> >> >
>> >> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> >> > index 876f1e3..89f014b 100644
>> >> > --- a/libavformat/avformat.h
>> >> > +++ b/libavformat/avformat.h
>> >> > @@ -1224,6 +1224,10 @@ void       av_stream_set_r_frame_rate(AVStream *s, AVRational r);
>> >> >  struct AVCodecParserContext *av_stream_get_parser(const AVStream *s);
>> >> >  char* av_stream_get_recommended_encoder_configuration(const AVStream *s);
>> >> >  void  av_stream_set_recommended_encoder_configuration(AVStream *s, char *configuration);
>> >> > +int64_t av_stream_get_cur_dts(AVStream *s);
>> >> > +int64_t av_stream_get_first_dts(AVStream *s);
>> >> > +int av_stream_get_pts_wrap_bits(AVStream *s);
>> >> > +int64_t av_stream_get_codec_info_nb_frames(AVStream *s);
>> >> >
>> >> >  /**
>> >> >   * Returns the pts of the last muxed packet + its duration
>> >> > diff --git a/libavformat/utils.c b/libavformat/utils.c
>> >> > index 6f343f2..5168816 100644
>> >> > --- a/libavformat/utils.c
>> >> > +++ b/libavformat/utils.c
>> >> > @@ -132,6 +132,27 @@ struct AVCodecParserContext *av_stream_get_parser(const AVStream *st)
>> >> >      return st->parser;
>> >> >  }
>> >> >
>> >> > +int64_t av_stream_get_cur_dts(AVStream *s)
>> >> > +{
>> >> > +    return s->cur_dts;
>> >> > +}
>> >> > +
>> >> > +int64_t av_stream_get_first_dts(AVStream *s)
>> >> > +{
>> >> > +    return s->first_dts;
>> >> > +}
>> >> > +
>> >> > +int av_stream_get_pts_wrap_bits(AVStream *s)
>> >> > +{
>> >> > +    return s->pts_wrap_bits;
>> >> > +}
>> >> > +
>> >> > +int64_t av_stream_get_codec_info_nb_frames(AVStream *s)
>> >> > +{
>> >> > +    return s->codec_info_nb_frames;
>> >> > +}
>> >> > +
>> >> > +
>> >>
>> >> erm. how about we move them in the public fields instead?
>> >
>> > The 3.0 release has these fields in one location, the 3.1 release
>> > has them in a different location because codecpar was added in the
>> > middle. The fields are marked as private
>> >
>> > To fix this we can either as in this patchset add accessors and
>> > use
>> > them in all applications (probably not just ours)
>> >
>> > OR
>> >
>> > we could move codecpar to the end of AVStream
>> >
>> > OR
>> >
>> > we could bump major abi version
>> >
>> > what do people prefer ?
>> >
>> > moving them into the pblic fields will not make them match where
>> > they where in 3.0, which is the problem  we have
>> >
>>
>> They are already in the wrong position, and they are marked explicitly
>> as private fields, so any app using them is doing it wrong, we should
>> not break the actual public ABI to cater to that.
>> Moving codecpar now seems like a bad solution, as in contrast to these
>> fields, codecpar is full public API/ABI and is in a release already
>> (even if its only a few days old).
>>
>
>> Adding new accessors will also not fix those old apps, since old
>> ffmpeg wouldnt have the accessor,
>
> Backporting the accessors is very simple
>
>
>> so its no better then any other
>> solution, and since we stopped caring about ABI compat with libav, the
>> primary reason for them has also vanished.
>> So I would vote for moving private fields that should be public right
>> above the private marker and be done with it.
>
> As this doesnt solve the ABI problem it would not help with this issue
> or the distributions who want (but cannot) ship FFmpeg 3.1
>
> Please tell me how you prefer the ABI issue with 3.0 / 3.1 to be
> fixed

I prefer not to, since the public ABI is not broken.
We have many contexts with a public and private part, and we have
added public fields above the private marker before. Whats different
in 3.1 that this is a problem suddenly?

We all know ffmpeg.c and ffplay.c violate the API/ABI in some places,
that should be fixed, but is not something "new" to 3.1

- Hendrik


More information about the ffmpeg-devel mailing list