[FFmpeg-devel] [PATCH] lavf/utils: update stream duration only if it is not set or 0

[Matthieu Bouron]

On Tue, Jul 04, 2017 at 05:24:45PM +0200, Matthieu Bouron wrote:
> On Tue, Jul 04, 2017 at 12:31:59PM +0200, Michael Niedermayer wrote:
> > On Mon, Jul 03, 2017 at 02:02:24PM +0200, Matthieu Bouron wrote:
> > > ---
> > > 
> > > The following patch makes lavf/utils only update stream duration only if it is
> > > not set or 0 in fill_all_stream_timings (which is called by
> > > avformat_find_stream_info).
> > > 
> > > In the context of mov demuxing, the patch makes the last packet duration the
> > > same as the one declared in the stts table for example (ie: we trust what the
> > > demuxer reports).
> > > 
> > > The patch passes fate, however it might not be valid but I would like to
> > > understand what is the purpose / use-case of the update_stream_timing /
> > > fill_all_stream_timings functions which are not too well documented.
> > 
> > I think these functions were mostly for filling in values that
> > have not been set otherwise
> > as in stream timings <-> file timings, that way demuxers can just
> > export what is stored in a file and teh rest is set from that
> > 
> > 
> > > 
> > > Thanks.
> > > 
> > > ---
> > >  libavformat/utils.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > > index 38d247c6cd..00adf026e6 100644
> > > --- a/libavformat/utils.c
> > > +++ b/libavformat/utils.c
> > > @@ -2654,7 +2654,8 @@ static void fill_all_stream_timings(AVFormatContext *ic)
> > >              if (ic->start_time != AV_NOPTS_VALUE)
> > >                  st->start_time = av_rescale_q(ic->start_time, AV_TIME_BASE_Q,
> > >                                                st->time_base);
> > > -            if (ic->duration != AV_NOPTS_VALUE)
> > > +            if (ic->duration != AV_NOPTS_VALUE &&
> > > +                (st->duration == 0 || st->duration == AV_NOPTS_VALUE))
> > >                  st->duration = av_rescale_q(ic->duration, AV_TIME_BASE_Q,
> > >                                              st->time_base);
> > >          }
> > 
> > can a inconsistency occur from setting start_time and duration from
> > different sources ?
> 
> Probably. Maybe the patch should *not* update the timings if both
> start_time and duration are already set ?

Nevermind, we only enter this block of code if st->start_time ==
AV_NOPTS_VALUE.

> 
> -- 
> Matthieu B.

-- 
Matthieu B.