[FFmpeg-devel] [PATCH] avoid infinite loop when seeking flv and non seekable input

Michael Niedermayer michaelni
Thu Jul 31 13:30:03 CEST 2008


On Wed, Jul 30, 2008 at 02:34:37PM -0700, Baptiste Coudurier wrote:
> Michael Niedermayer wrote:
> > On Sun, Jul 27, 2008 at 05:08:47PM -0700, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >>> On Sat, Jul 26, 2008 at 09:56:17PM -0700, Baptiste Coudurier wrote:
> >>>> Michael Niedermayer wrote:
> >>>>> On Wed, Jul 23, 2008 at 08:35:49PM -0700, Baptiste Coudurier wrote:
> >>>>>> Hi Michael,
> >>>>>>
> >>>>>> Michael Niedermayer wrote:
> >>>>>>> On Tue, Jul 22, 2008 at 07:41:51PM -0700, Baptiste Coudurier wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> $subject,
> >>>>>>>>
> >>>>>>>> Reproduceable when trying to seek and input is non seekable (like http
> >>>>>>>> through ffserver).
> >>>>>>>>
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2705408, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2763520, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2818048, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2856192, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2889728, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 0, size 11469089, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2949120, flags 0
> >>>>>>>> [flv @ 0x8e27eb0]skipping flv packet: type 32, size 2987264, flags 0
> >>>>>>>>
> >>>>>>>> With patch resync happens faster:
> >>>>>>> I dont understand the problem, "trying to seek and input is non seekable"
> >>>>>>> makes no sense to me. If the input is unseekable, seeking wont succeed
> >>>>>>> anyway.
> >>>>>> It will it when seeking forward, aviobuf.c:
> >>>>>> } else if(s->is_streamed && !s->write_flag &&
> >>>>>>        offset1 >= 0 && offset1 < (s->buf_end - s->buffer) + (1<<16)){
> >>>>> indeed
> >>>>> this could be fixed by removing  "offset1 < (s->buf_end - s->buffer) + (1<<16)"
> >>>>> or increasing (1<<16)
> >>>>> feel free to change that.
> >>>>>
> >>>> I checked more deeply and here comes patches.
> >>>>
> >>>> av_read_frame_flush needs to be done after seek in case of failure, this
> >>>> hunk will be applied before and separately.
> >>>>
> >>>> url_fseek is checked for error, and when is_streamed is set but seeking
> >>>> forward is not possible, return error.
> >>> [...]
> >>>
> >>>> @@ -1465,14 +1466,14 @@
> >>>>      if (index < 0)
> >>>>          return -1;
> >>>>  
> >>>> -    av_read_frame_flush(s);
> >>>>      if (s->iformat->read_seek){
> >>>>          if(s->iformat->read_seek(s, stream_index, timestamp, flags) >= 0)
> >>>>              return 0;
> >>>>      }
> >>>>      ie = &st->index_entries[index];
> >>>>      if ((ret = url_fseek(s->pb, ie->pos, SEEK_SET)) < 0)
> >>>>          return ret;
> >>>> +    av_read_frame_flush(s);
> >>>>      av_update_cur_dts(s, st, ie->timestamp);
> >>>>  
> >>>>      return 0;
> >>> Does this pass the regression tests?
> >> Yes.
> >>
> >>> What concerns me is that av_read_frame_flush() is not done anymore when
> >>> s->iformat->read_seek is set and it succeeds
> >> Yes, but I don't understand the code either, iformat->read_seek will be
> >> called before
> >> calling av_seek_frame_generic anyway, and it would have failed.
> >>
> > 
> >> It seems to me that seeking internals needs to be cleaned up,
> > 
> > yes
> > 
> > 
> >> personnally I would not try generic seeking if native failed, and I even
> >> sent a patch for this.
> > 
> > hmm, my memory is rusty, do you remember the subject or date?
> > Maybe the fallback should be removed but id like to first check up the
> > past discussions
> 
> Yes, [Ffmpeg-devel] [RFC] av_seek_frame behaviour, 21/04/07, indeed I
> let this rotting for some time....
> 
> My last enumeration of seeking behaviour is still valid, and indeed your
> remove flv seek function on 14/04/08, sorry for that.

hmm, iam not against removing the fallback if someone fixes any bugs that
might uncover.


> 
> >> Also I also wondered what was "timestamp" supposed to be, dts or pts ?
> >> Because mpeg ps returns dts for read_timestamps.
> > 
> > This is all rather tricky
> > read_timestamps() is used for binary searching, which requires the file
> > position - timestamp to be monotone, pts are not monotone due to B frames.
> > thus as it is it has to return dts or it will behave somewhat random.
> > But then it doesnt behave very well for mpeg anyway because of timestamp
> > discontinuities.
> > So for mpeg we really should do some sanity checks about monotonicity
> > and the byte vs timestamp distance the final seek achived, and if it looks
> > odd fall back to seeking based on strict CBR assumption.
> > 
> > or
> > 
> > we could try to first seek based on strict CBR assumption, that is
> > current_pos + (wanted_time - curren_time)*bitrate
> > then fetch a dts from the new position and apply above formular again
> > and if the new seek would be over a shorter distance than the previos
> > repeat. If not assume that we went over a timestamp discontinuity and
> > just accept the current position.
> > 
> > patch very welcome for this!
> 
> I'll try to code something, but Im not sure if for now mpeg seeking is
> really problematic, is it ?

Well, it should fail pretty complete with a file with timestamp discontinuity
(can easily be generated by cat file1.mpg file2.mpg)

Of course there are other possible more important bugs in the seeking code ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080731/807b62fb/attachment.pgp>



More information about the ffmpeg-devel mailing list