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?
> -int av_write_frame(AVFormatContext *s, AVPacket *pkt)
> +static int prepare_input_packet(AVFormatContext *s, AVPacket *pkt)
> {
> int ret;
would it be a problem to move the prepare_input_packet() factorization
to a separate patch?
> @@ -389,16 +406,70 @@ int av_write_frame(AVFormatContext *s, AVPacket *pkt)
> if (ret < 0)
> return ret;
>
> +#if !FF_API_COMPUTE_PKT_FIELDS2
> + /* sanitize the timestamps */
> + if (!(s->oformat->flags & AVFMT_NOTIMESTAMPS)) {
> + AVStream *st = s->streams[pkt->stream_index];
> +
> + /* when there is no reordering (so dts is equal to pts), but
> + * only one of them is set, set the other as well */
> + if (!st->internal->reorder) {
> + if (pkt->pts == AV_NOPTS_VALUE && pkt->dts != AV_NOPTS_VALUE)
> + pkt->pts = pkt->dts;
> + if (pkt->dts == AV_NOPTS_VALUE && pkt->pts != AV_NOPTS_VALUE)
> + pkt->dts = pkt->pts;
> + }
> +
> + /* check that the timestamps are set */
> + if (pkt->pts == AV_NOPTS_VALUE || pkt->dts == AV_NOPTS_VALUE) {
> + av_log(s, AV_LOG_ERROR,
> + "Timestamps are unset in a packet for stream %d\n",
> st->index);
> + return AVERROR(EINVAL);
> + }
> +
> + /* check that the dts are increasing (or at least non-decreasing,
> + * if the format allows it */
> + if (st->cur_dts != AV_NOPTS_VALUE &&
> + ((!(s->oformat->flags & AVFMT_TS_NONSTRICT) && st->cur_dts >=
> pkt->dts) ||
> + st->cur_dts > pkt->dts)) {
> + av_log(s, AV_LOG_ERROR,
> + "Application provided invalid, non monotonically
> increasing "
> + "dts to muxer in stream %d: %" PRId64 " >= %" PRId64 "\n",
> + st->index, st->cur_dts, pkt->dts);
> + return AVERROR(EINVAL);
> + }
> +
> + if (pkt->pts < pkt->dts) {
> + av_log(s, AV_LOG_ERROR, "pts %" PRId64 " < dts %" PRId64 " in
> stream %d\n",
> + pkt->pts, pkt->dts, st->index);
> + return AVERROR(EINVAL);
> + }
> + }
> +#endif
> +
> + return 0;
> +}
> +
> +int av_write_frame(AVFormatContext *s, AVPacket *pkt)
> +{
> + int ret;
> +
> + ret = prepare_input_packet(s, pkt);
> + if (ret < 0)
> + return ret;
> +
> if (!pkt) {
> if (s->oformat->flags & AVFMT_ALLOW_FLUSH)
> return s->oformat->write_packet(s, pkt);
> return 1;
> }
>
> +#if FF_API_COMPUTE_PKT_FIELDS2
> ret = compute_pkt_fields2(s, s->streams[pkt->stream_index], pkt);
>
> if (ret < 0 && !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
> return ret;
> +#endif
>
> ret = write_packet(s, pkt);
>
> @@ -552,7 +623,7 @@ int av_interleaved_write_frame(AVFormatContext *s,
> AVPacket *pkt)
> {
> int ret, flush = 0;
>
> - ret = check_packet(s, pkt);
> + ret = prepare_input_packet(s, pkt);
> if (ret < 0)
> goto fail;
>
> @@ -561,8 +632,10 @@ int av_interleaved_write_frame(AVFormatContext *s,
> AVPacket *pkt)
>
> av_log(s, AV_LOG_TRACE, "av_interleaved_write_frame size:%d dts:%"
> PRId64 " pts:%" PRId64 "\n",
> pkt->size, pkt->dts, pkt->pts);
> +#if FF_API_COMPUTE_PKT_FIELDS2
> if ((ret = compute_pkt_fields2(s, st, pkt)) < 0 &&
> !(s->oformat->flags & AVFMT_NOTIMESTAMPS))
> goto fail;
> +#endif
>
> if (pkt->dts == AV_NOPTS_VALUE && !(s->oformat->flags &
> AVFMT_NOTIMESTAMPS)) {
> ret = AVERROR(EINVAL);
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 734f83f..bb17b4a 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2460,6 +2460,8 @@ static void free_stream(AVStream **pst)
> if (st->attached_pic.data)
> av_packet_unref(&st->attached_pic);
>
> + av_freep(&st->internal);
> +
> av_dict_free(&st->metadata);
> av_freep(&st->probe_data.buf);
> av_free(st->index_entries);
> @@ -2551,6 +2553,11 @@ AVStream *avformat_new_stream(AVFormatContext *s,
> const AVCodec *c)
> av_free(st);
> return NULL;
> }
> +
> + st->internal = av_mallocz(sizeof(*st->internal));
> + if (!st->internal)
> + goto fail;
> +
> if (s->iformat) {
> /* no default bitrate if decoding */
> st->codec->bit_rate = 0;
> @@ -2583,6 +2590,9 @@ AVStream *avformat_new_stream(AVFormatContext *s, const
> AVCodec *c)
>
> s->streams[s->nb_streams++] = st;
> return st;
> +fail:
> + free_stream(&st);
> + return NULL;
> }
>
> AVProgram *av_new_program(AVFormatContext *ac, int id)
> 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?
--
Vittorio
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
