[FFmpeg-devel] [PATCH] Set the eof flag if a fskip call fails

[Martin Storsjö]

On Mon, 19 Jul 2010, Michael Niedermayer wrote:

> On Mon, Jul 19, 2010 at 02:59:55PM +0300, Martin Storsj? wrote:
> > On Mon, 19 Jul 2010, Michael Niedermayer wrote:
> > 
> > > On Mon, Jul 19, 2010 at 12:22:16PM +0300, Martin Storsj? wrote:
> > > > Hi,
> > > > 
> > > > If the user called url_fskip and it failed, the read position of the 
> > > > stream is not where the user intended it to be, and I think we can assume 
> > > > that we hit eof. Thus, setting eof_reached = 1 seems logical to me.
> > > 
> > > how is this differernt from url_fseek() ?
> > 
> > I'd say, with fseek, one may be ready to fallback to not doing something 
> > if the seek failed, but if fskip failed, we're probably at the end of the 
> > stream, and need to take some action (stop reading), otherwise we're not 
> > at least reading at the position we think we are.
> > 
> > I guess I should explain the initial reason for this. I recently debugged 
> > a case where the flv demuxer went out of sync with the data it read, since 
> > one of the url_fseek()s (that at that point used an ordinary low-level 
> > http seek, just for skipping 4 bytes ahead) spuriously failed. This lead 
> > to the demuxer suddenly returning garbage packets since it wasn't reading 
> > at the offset it thought it was, since the seek had failed, but we weren't 
> > yet at eof.
> > 
> > So, I'm just trying to add proper error checking at each level so noone 
> > else will need to go through the same debugging session as I did.
> > 
> > Now we at least can notice failures from url_fskip since it returns an 
> > error code, but instead of checking each of them, having them set the eof 
> > flag would make the error handling much leaner, and it would fit into how 
> > at least I conceptually see fskip.
> > 
> > Also, the short seek fix for aviobuf makes this issue almost completely go 
> > away (since a short seek done by a read will trigger the eof flag).
> 
> is your reply somehow related to my question?

Yes, it was an attempt at showing the reasoning why I thought this would 
be valuable.

> again, id like to know why fskip (which is nothing but a shorthand
> form of fseek(cur)) should behave differently from fseek(cur)

If you think of fskip as fseek(cur), then yes, it should behave in the 
same way, not setting the eof flag (unless it happens to do the seek by 
reading, when it actually is set). If you instead think of fskip as 
"advance the read pointer by reading x bytes but do not return them", 
setting the eof flag if it fails would be the semantically correct thing.

// Martin