[FFmpeg-devel] [PATCH] clear s->error in avio_read
hubbe at google.com
Fri Mar 24 19:47:53 EET 2017
On Fri, Mar 24, 2017 at 12:47 AM, wm4 <nfxjfg at googlemail.com> wrote:
> On Mon, 20 Mar 2017 14:39:07 -0700
> Fredrik Hubinette <hubbe-at-google.com at ffmpeg.org> wrote:
> > It looks like the value in s->error also comes from an earlier call to
> > avio_read().
> > ogg_read_page tries to read 4439 bytes from the file. It has 524 bytes
> > already buffered.
> > fill_buffer() fails with an EIO, but because of the buffered bytes,
> > avio_read()
> > returns 524 and leaves the EIO in s->error.
> Trying to understand this. Is this an EOF situation? As in, it tries to
> read more bytes than there are left in the stream. In that case, there
> should probably be no EIO, but AVERROR_EOF (and should possibly handled
> specially if "short reads" are allowed in this case).
> It might also be that I'm missing something completely.
I think it is. We have a custom IO layer in chrome which currently always
returns EIO for all unexpected errors. (Including EOF I think).
I can try changing our code to return AVERROR_EOF when no bytes are read.
That will cause s->error to be AVERROR_EOF and the
avio_read() code will end up returning AVERROR_EOF in the second case.
However, it is my opinion that it would return the correct
value by accident.
> error should not be cleared by read so that the "user" can check for
> errors after a sequence of reads without needing to check after
> each individual read. Some demuxers do this that would break if
> error is reset here
Ok, I couldn't find a description of how s->error is supposed to work.
I have two questions:
Is it reasonable for avio_read to return s->error instead of AVERROR_EOF
because s->error was already set when avio_read was called? (If not, I can
patch this by adding return values from fill_buffer.)
Is it reasonable that a *successful* call to the ogg/webm demuxer sets
s->error? (If not, maybe s->error should be cleared there?)
My interpretation of s->error is that it works like errno(3), and should be
ignored unless a return value indicate an error.
That seems to be what a lot of the code does, but it is not what
More information about the ffmpeg-devel