On Mon, Mar 20, 2017 at 02:49:18PM -0700, Fredrik Hubinette wrote: > Hopefully valid patch attached. > > > On Mon, Mar 20, 2017 at 2:39 PM, Fredrik Hubinette <hu...@google.com> 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. > > > > /Fredrik "Hubbe" Hubinette > > > > > > On Mon, Mar 20, 2017 at 1:53 PM, Fredrik Hubinette <hu...@google.com> > > wrote: > > > >> > >> On Mon, Mar 20, 2017 at 1:35 PM, Michael Niedermayer < > >> mich...@niedermayer.cc> wrote: > >> > >>> On Mon, Mar 20, 2017 at 10:21:08AM -0700, Fredrik Hubinette wrote: > >>> > In some cases (when parsing OGG) non-fatal errors can happen, which > >>> > will cause s->error to be set. In most cases, this is not a problem > >>> beucase > >>> > s->error is not checked unless an actual error has occurred. However, > >>> > when avio_read() fails to read more bytes, it checks s->error to > >>> decide if > >>> > it just reached the end of the file, or an error occurred. Since > >>> s->error is > >>> > not modified if no error occurred, this is not reliable unless we first > >>> > clear > >>> > s->error before reading. > >>> > > >>> > --- > >>> > libavformat/aviobuf.c | 2 ++ > >>> > > >>> > 2 files changed, 7 insertions(+) > >>> > >>> how can the issue you describe be reproduced > >>> > >>> > >> I don't currently have a simple repro. I discovered it while trying to > >> add some buffering > >> between the demuxer and decoder in chrome. One of our tests fails, and > >> when > >> looked into it, I discovered that s->error was already 5 (EIO) when > >> entering > >> avio_read. I have not yet tracked down where the EIO came from > >> originally. > >> > >> also thats not a valid git patc > >>> > >> > >> Ops, I had a chrome patch and I removed the chrome-specific parts, but > >> I must have messed it up. I will make a better patch. > >> > >> > >>> [...] > >>> -- > >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > >>> > >>> Those who are best at talking, realize last or never when they are wrong. > >>> > >>> _______________________________________________ > >>> ffmpeg-devel mailing list > >>> ffmpeg-devel@ffmpeg.org > >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >>> > >>> > >> > >
> aviobuf.c | 2 ++
> 1 file changed, 2 insertions(+)
> 3ef8ffcafab28750ede89a2b7390203a95afb59f
> 0001-clear-s-error-in-avio_read.patch
> From a26c186d2ac6d013215e9707fa3508ba1ef8537a Mon Sep 17 00:00:00 2001
> From: Fredrik Hubinette <hu...@google.com>
> Date: Mon, 20 Mar 2017 14:47:06 -0700
> Subject: [PATCH] clear s->error in avio_read
>
> ---
> libavformat/aviobuf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 4ade4d0d7e..0912dfdb57 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -615,6 +615,8 @@ int avio_read(AVIOContext *s, unsigned char *buf, int
> size)
> {
> int len, size1;
>
> + s->error = 0;
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
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
