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

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at gmx.de> writes:

> 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 fix the documentation.  I don't understand what you are complaining
about.

> 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...

The protocol seek function _is never called_ in this case.  No, the
documentation of url_fseek() doesn't mention this.  Patches welcome.

-- 
M?ns Rullg?rd
mans at mansr.com