[FFmpeg-devel] [PATCH 3/3] avformat/mov: be more tolerant with interleaving when seeking is slow

[wm4]

On Wed, 26 Mar 2014 20:42:37 +0100
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> On Wed, Mar 26, 2014 at 08:18:22PM +0100, Michael Niedermayer wrote:
> > On Wed, Mar 26, 2014 at 07:34:28PM +0100, Reimar Döffinger wrote:
> > > On Wed, Mar 26, 2014 at 05:59:02PM +0100, Michael Niedermayer wrote:
> > > > ---
> > > >  libavformat/mov.c |    5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/libavformat/mov.c b/libavformat/mov.c
> > > > index 469da94..2a045d5 100644
> > > > --- a/libavformat/mov.c
> > > > +++ b/libavformat/mov.c
> > > > @@ -3519,12 +3519,13 @@ static AVIndexEntry *mov_find_next_sample(AVFormatContext *s, AVStream **st)
> > > >          if (msc->pb && msc->current_sample < avst->nb_index_entries) {
> > > >              AVIndexEntry *current_sample = &avst->index_entries[msc->current_sample];
> > > >              int64_t dts = av_rescale(current_sample->timestamp, AV_TIME_BASE, msc->time_scale);
> > > > +            int max_distance = (s->pb->seekable & AVIO_SEEKABLE_SLOW) ? 3 : 1;
> > > >              av_dlog(s, "stream %d, sample %d, dts %"PRId64"\n", i, msc->current_sample, dts);
> > > >              if (!sample || (!s->pb->seekable && current_sample->pos < sample->pos) ||
> > > >                  (s->pb->seekable &&
> > > >                   ((msc->pb != s->pb && dts < best_dts) || (msc->pb == s->pb &&
> > > > -                 ((FFABS(best_dts - dts) <= AV_TIME_BASE && current_sample->pos < sample->pos) ||
> > > > -                  (FFABS(best_dts - dts) > AV_TIME_BASE && dts < best_dts)))))) {
> > > > +                 ((FFABS(best_dts - dts) <= AV_TIME_BASE*max_distance && current_sample->pos < sample->pos) ||
> > > > +                  (FFABS(best_dts - dts) >  AV_TIME_BASE*max_distance && dts < best_dts)))))) {
> > > 
> > > Is it really a good thing to have two cases here?
> > > I'm tempted to say it only makes testing harder, since after this change
> > > any application needs to handle the larger desync to work with networked
> > > streams anyway, so what is the advantage of keeping the old limit?
> > 
> > currently if with networked files you are above the threshold
> > chances are the latency of seeking over the network will make
> > playback impossible
> > That patch is trying to fix that without breaking other things
> > changing the threshold unconditionally could break applications that
> > cant handle larger sync differences.
> 
> Well, it will still break those applications if they were previously
> used with a fast network link for example.
> Maybe this patch is indeed the pragmatic solution, it just doesn't
> feel great.

Making demuxing dependent on network speed seems to be a horrible
"solution". The only good thing about this is that it's not actually
network speed, but just a flag. On the other hand, I'm not quite sure
what this code does, so maybe I'm not qualified to comment. (Though I
still wonder whether as a user I should set this flag or not. And what
does it have to do with networking? Fast, local connections do exist.)

> > If someone has a better suggestion ?
> 
> At least me not really, it just seems worse long-term to have two modes of operation.