On Sun, 20 Sep 2020, Marton Balint wrote:
On Sun, 20 Sep 2020, Paul B Mahol wrote:
On Sun, Sep 20, 2020 at 03:16:15PM +0200, Marton Balint wrote:
On Sun, 20 Sep 2020, Paul B Mahol wrote:
> On Sun, Sep 20, 2020 at 10:52:52AM +0200, Marton Balint wrote:
> > Signed-off-by: Marton Balint <c...@passwd.hu>
> > ---
> > libavformat/aviobuf.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> > index 9675425349..aa1d6c0830 100644
> > --- a/libavformat/aviobuf.c
> > +++ b/libavformat/aviobuf.c
> > @@ -999,9 +999,12 @@ int ffio_ensure_seekback(AVIOContext *s, int64_t
buf_size)
> > int filled = s->buf_end - s->buffer;
> > ptrdiff_t checksum_ptr_offset = s->checksum_ptr ? s->checksum_ptr
- s->buffer : -1;
> >
> > - buf_size += s->buf_ptr - s->buffer + max_buffer_size;
> > + if (buf_size <= s->buf_end - s->buf_ptr)
> > + return 0;
> > +
> > + buf_size += s->buf_ptr - s->buffer + max_buffer_size - 1;
> >
> > - if (buf_size < filled || s->seekable || !s->read_packet)
> > + if (buf_size <= s->buffer_size || s->seekable || !s->read_packet)
> > return 0;
> > av_assert0(!s->write_flag);
>
>
> Not acceptable change.
>
> Your code does this to chained ogg files:
>
> XXX 10
> XXX 65307
> XXX 65307
> 102031
> 106287
> 110527
> 114745
> 119319
> [...]
This was also the case before the patch, no? So this alone is no reason
to reject the patch.
Exactly the reson for patch rejection is that it does not improve code at
all.
It might not fix your issue, but it certainly improves the code.
Ping for this as well.
Thanks,
Marton
>
> It continues allocating excessive small extra chunks of bytes for no
apparent reason in each and every call
> which slowly and gradually increases memory usage, but every call causes
unnecessary memcpy calls thus causing
> almost exponential slowdown of file processing.
And when I say ffio_ensure_seekback() has a design issue, this is exactly
what I mean. It is not that suprising, consider this:
I have 32k buffer, and I read at most 32k at once.
I want seekback of 64k. Buffer got increased to 96k
I read 64k.
I want seekback of 64k. Buffer got increased to 160k.
I read 64k.
... and so on.
My patch exactly does that and it proves it works.
a read call cannot flush the buffer because I am always whitin my
requested
boundary. ffio_ensure_seekback() cannot flush the buffer either, because
it
is not allowed to do that. Therefore I consume infinite memory.
This explanation is flawed.
Please point out where the flaw is.
Thanks,
Marton
>
> Lines with XXX, means that allocation and memcpy was not needed.
Are you sure about that? Feel free to give an example with buffer sizes
and
buffer positions, and prove that reallocation is uneeded. But please be
aware how fill_buffer() works and make sure that when reading sequentially
up to buf_size, seeking within the current pos and pos+buf_size is
possible.
Seeking should be possible anywhere between buffer start and buffer end.
current buffer ptr is not important as it just points within buffer.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
