[MPlayer-dev-eng] Re: [PATCH] Fix some seek failure

[Ivan Kalvachev]

2006/3/15, Alban Bedel <albeu at free.fr>:
> On Tue, 14 Mar 2006 10:22:17 +0200
> "Ivan Kalvachev" <ikalvachev at gmail.com> wrote:
>
> > 2006/3/13, Alban Bedel <albeu at free.fr>:
> > >
> > > Hi,
> > >
> > > their is a little bug in stream_seek() which can lead to some
> > > strange seeking failure, in particular with ftp.
> > > Seeks are always aligned on the buffer size, then a single
> > > stream_fill_buffer is made and failure is returned if the wanted
> > > position is not in the buffer. So that obviously can fail if the
> > > call to stream_fill_buffer() doesn't fill the whole buffer (it
> > > doesn't have to).
> > [..]
> >
> > Are you sure it doesn't have to?
> >
> > I think stream_fill_buffer is always supposed to return full buffer,
> > unless EOF is reached (then it returns only the available data). It is
> > used this way on few other places (e.g. cache.c) and they would need
> > fixing. It would be better to fix it in stream_fill_buffer or t
>
> I double checked and that should be ok. stream_fill_buffer() is an
> "internal" function. It is only called from stream_seek_long() and
> cache_stream_fill_buffer(). The cache use stream_read() to fill his
> buffer, so no pb there.

The function name suggest that it fills the buffer, it is not named
"fill_some_data_if_you_have_pleace" :)

Wont it be better to fix it inside stream_fill_buffer(), compared to
make same fix on 2 (or more) places?

Hum... let me count the occurances (ignoring the calling context)
cache.c:324
stream.c:308
stream.c:327
stream.h:111 (that binds it to cache_stream_fill_buffer so lines
bellow are also added...)
stream.h:117
stream.h:200
stream.h243

Anyway I think that this fix is actually workaround of bug that is
somewhere else. For example it could be that ftp streaming returns
early after reading of single packet without filling the requested
data. MPlayer will hang eiher in ftp code or in your fix. That's why
stream reading is done in cache fork.

(recv flag MSG_WAITALL may be the right way to fix ftp).