Message ID | 20170703120224.25003-1-matthieu.bouron@gmail.com |
---|---|
State | New |
Headers | show |
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 ? [...]
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 ?
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.
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); }