[libav-devel] [PATCH 4/4] lavf: deprecate compute_pkt_fields2

Vittorio Giovara vittorio.giovara at gmail.com
Sun Nov 8 20:39:18 CET 2015


On Sun, Nov 8, 2015 at 8:30 PM, Anton Khirnov <anton at khirnov.net> wrote:
> Quoting Vittorio Giovara (2015-11-08 20:19:05)
>> On Sun, Nov 8, 2015 at 11:54 AM, Anton Khirnov <anton at khirnov.net> wrote:
>> > All encoders set pts and dts properly now (and have been doing that for
>> > a while), so there is no good reason to do any timestamp guessing in the
>> > muxer.
>> > ---
>> >  libavformat/avformat.h |  8 ++++++
>> >  libavformat/internal.h | 12 ++++++++
>> >  libavformat/mux.c      | 77 ++++++++++++++++++++++++++++++++++++++++++++++++--
>> >  libavformat/utils.c    | 10 +++++++
>> >  libavformat/version.h  |  3 ++
>> >  5 files changed, 108 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> > index 470bbc6..e441486 100644
>> > --- a/libavformat/avformat.h
>> > +++ b/libavformat/avformat.h
>> > @@ -688,6 +688,8 @@ typedef struct AVIndexEntry {
>> >   */
>> >  #define AV_DISPOSITION_ATTACHED_PIC      0x0400
>> >
>> > +typedef struct AVStreamInternal AVStreamInternal;
>> > +
>> >  /**
>> >   * Stream structure.
>> >   * New fields can be added to the end with minor version bumps.
>> > @@ -878,6 +880,12 @@ typedef struct AVStream {
>> >                                      support seeking natively. */
>> >      int nb_index_entries;
>> >      unsigned int index_entries_allocated_size;
>> > +
>> > +    /**
>> > +     * An opaque field for libavformat internal usage.
>> > +     * Must not be accessed in any way by callers.
>> > +     */
>> > +    AVStreamInternal *internal;
>> >  } AVStream;
>> >
>> >  #define AV_PROGRAM_RUNNING 1
>> > diff --git a/libavformat/internal.h b/libavformat/internal.h
>> > index a65a3b7..7bbf775 100644
>> > --- a/libavformat/internal.h
>> > +++ b/libavformat/internal.h
>> > @@ -90,6 +90,18 @@ struct AVFormatInternal {
>> >       * Timebase for the timestamp offset.
>> >       */
>> >      AVRational offset_timebase;
>> > +
>> > +#if FF_API_COMPUTE_PKT_FIELDS2
>> > +    int missing_ts_warning;
>> > +#endif
>> > +};
>> > +
>> > +struct AVStreamInternal {
>> > +    /**
>> > +     * Set to 1 if the codec allows reordering, so pts can be different
>> > +     * from dts.
>> > +     */
>> > +    int reorder;
>> >  };
>>
>> Is this new private field necessary? if reordering is the only
>> usecase, couldn't we just check the AV_CODEC_PROP_REORDER flag from
>> avctx->codec?
>
> There is no avctx->codec available to the muxer.

What about avstream->codec?

> This is a property of the codec id, so we either have to look up the
> corresponding codec descripter at each call, or do what I did.

Yes, this approach is better than a lookup, but it's still adding a
private structure for just a boolean value.
Wouldn't an avstream side data be less invasive anyway?

>> > diff --git a/libavformat/version.h b/libavformat/version.h
>> > index d74968a..d004a55 100644
>> > --- a/libavformat/version.h
>> > +++ b/libavformat/version.h
>> > @@ -60,5 +60,8 @@
>> >  #ifndef FF_API_LAVF_FMT_RAWPICTURE
>> >  #define FF_API_LAVF_FMT_RAWPICTURE      (LIBAVFORMAT_VERSION_MAJOR < 58)
>> >  #endif
>> > +#ifndef FF_API_COMPUTE_PKT_FIELDS2
>> > +#define FF_API_COMPUTE_PKT_FIELDS2      (LIBAVFORMAT_VERSION_MAJOR < 58)
>> > +#endif
>>
>> I might be missing something, but aren't FF_API defines only for
>> public fields? This is mostly private so you could change the code
>> right away couldn't you?
>
> I could, but callers that do not set the timestamps properly would
> break. Since we supported that until now, it makes more sense to
> deprecate it first.

ok

-- 
Vittorio


More information about the libav-devel mailing list