[FFmpeg-devel] Fix for aviobuf.c::fill_buffer

[code bythepound]

I will see if I can put together a test case that exposes the problem.  The
comment for this block says "make buffer smaller in case it ended up large
after probing" - I'm not 100% certain what that means.  I put prints inside
this block and after the ffio_set_buf_size() call, this block is never
executed again.  In my custom case I am streaming an MPEG.TS file and I
don't know if this involves this extra "probing" or not.

Even so, there is nothing in the code above that would limit 'len' to be
greater than the size of s->orig_buffer_size, so the assert doesn't seem
like the right way to handle this here.

On Wed, Jul 20, 2016 at 4:41 PM Michael Niedermayer <michael at niedermayer.cc>
wrote:

> On Wed, Jul 20, 2016 at 03:59:39PM +0000, code bythepound wrote:
> > My app seems to hit the av_assert0(len >= s->orig_buffer_size) (line 535
> in
> > my sources) in aviobuf.c::fill_buffer.  FWIW I have registered a custom
> > AVIOContext that will consistently not fill the entire buffer when the
> read
> > callback is called.
> >
> > The outcome is that 'len' in fill_buffer is decremented with each call
> > until it is less than s->orig_buffer_size and will assert fail.  It seems
> > that instead of the assertion failure, the method should be:
> >
> > if( len >= s->orig_buffer_size )
> >     len = s->orig_buffer_size
> > // otherwise, len is < than orig_buffer_size, but is sized correctly to
> > fill remainder of buffer.
> >
> > On the next iteration, the previous clause (dst == s->buffer) is executed
> > and the buffer is reset.  After that, this block is never executed again.
> >
> > With the above fix in, my app seems to work fine.
> >
> > Since the fill_buffer code has been the same for many releases of ffmpeg,
> > I'm wondering how I could be the first to notice it, and if there is
> > something else I could be doing wrong to cause this?
>
> is there some easy way to reproduce this ?
> i tried
> diff --git a/libavformat/file.c b/libavformat/file.c
> index 264542a..529a975 100644
> --- a/libavformat/file.c
> +++ b/libavformat/file.c
> @@ -108,6 +108,7 @@ static int file_read(URLContext *h, unsigned char
> *buf, int size)
>  {
>      FileContext *c = h->priv_data;
>      int ret;
> +    size = FFMIN(size, size/5 + 1);
>      size = FFMIN(size, c->blocksize);
>      ret = read(c->fd, buf, size);
>      if (ret == 0 && c->follow)
>
> but it doesnt trigger the assert
>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Modern terrorism, a quick summary: Need oil, start war with country that
> has oil, kill hundread thousand in war. Let country fall into chaos,
> be surprised about raise of fundamantalists. Drop more bombs, kill more
> people, be surprised about them taking revenge and drop even more bombs
> and strip your own citizens of their rights and freedoms. to be continued
>