[FFmpeg-cvslog] r22197 - trunk/libavformat/matroskaenc.c

[Reimar Döffinger]

On Thu, Mar 04, 2010 at 06:12:42PM +0000, M?ns Rullg?rd wrote:
> Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:
> 
> > On Thu, Mar 04, 2010 at 09:53:01AM +0100, conrad wrote:
> >> Author: conrad
> >> Date: Thu Mar  4 09:53:00 2010
> >> New Revision: 22197
> >> 
> >> Log:
> >> Attempt seeking to write EBML master sizes even if streamed
> >> 
> >> Most EBML masters are much smaller than IO_BUFFER_SIZE and thus the size
> >> can be updated. This makes parsing the resulting files easier.
> >> 
> >> Modified:
> >>    trunk/libavformat/matroskaenc.c
> >> 
> >> Modified: trunk/libavformat/matroskaenc.c
> >> ==============================================================================
> >> --- trunk/libavformat/matroskaenc.c	Thu Mar  4 05:58:43 2010	(r22196)
> >> +++ trunk/libavformat/matroskaenc.c	Thu Mar  4 09:53:00 2010	(r22197)
> >> @@ -218,11 +218,8 @@ static void end_ebml_master(ByteIOContex
> >>  {
> >>      int64_t pos = url_ftell(pb);
> >>  
> >> -    // leave the unknown size for masters when streaming
> >> -    if (url_is_streamed(pb))
> >> +    if (url_fseek(pb, master.pos - master.sizebytes, SEEK_SET) < 0)
> >>          return;
> >
> > Is this "misuse" of fseek covered by our API specification?
> > It assumes that seeking with url_is_streamed does never have a
> > negative effect, however I'm not sure it couldn't be horribly
> > slow or it might not keep the state the same or it might be possible
> > to seek backward but seeking forward to the original position might fail...
> 
> Seeking within the buffer is always fast.  The seek function of the
> underlying protocol is only invoked if the target is outside the buffer.

I don't understand what that has to do with what I said at all.
There's nothing in the documented API that would allow you to assume
that url_fseek will not seek outside the buffer if url_is_streamed
is true.
So above code adds the assumption that seeking backwards will be either
reasonably fast or fail even in the url_is_streamed case, while the
documentation would not even give someone implementing a protocol any hint
about this - they might reasonably say "well, seek is so horribly slow
we absolutely need avoid it, so we set is_streamed, however we would still
be able to somehow play formats that absolutely require seeking, so we implement
it anyway". If they wanted it to work well with the matroska muxer too they
now would have to add a special hack...