On Sun, Nov 8, 2015 at 8:44 PM, Anton Khirnov <[email protected]> wrote: > Quoting Vittorio Giovara (2015-11-08 20:39:18) >> On Sun, Nov 8, 2015 at 8:30 PM, Anton Khirnov <[email protected]> wrote: >> > Quoting Vittorio Giovara (2015-11-08 20:19:05) >> >> On Sun, Nov 8, 2015 at 11:54 AM, Anton Khirnov <[email protected]> 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? > > You cannot rely on it containing an actual AVCodec. And in any case, > it's going away Soon(tm). > >> >> > 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. > > It's not just for this field. In the future, a lot more stuff will move > there. For example, everything that now lives in AVStream itself and is > marked as private. > >> Wouldn't an avstream side data be less invasive anyway? > > Most certainly not. Side data is for sharing data between callers and > various libav components, not for storing internal state.
fair enough, I withdraw my objections but it'd be nice that you briefly mention the future use case and planned changes in the commit log. -- Vittorio _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
