[FFmpeg-devel] [PATCH] clear s->error in avio_read

[Fredrik Hubinette]

ping?


On Mon, Mar 20, 2017 at 2:49 PM, Fredrik Hubinette <hubbe at google.com> wrote:

> Hopefully valid patch attached.
>
>
> On Mon, Mar 20, 2017 at 2:39 PM, Fredrik Hubinette <hubbe at 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 <hubbe at google.com>
>> wrote:
>>
>>>
>>> On Mon, Mar 20, 2017 at 1:35 PM, Michael Niedermayer <
>>> michael at 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 at ffmpeg.org
>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>>
>>>
>>
>