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.
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.
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.
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.
Regards,
Marton
_______________________________________________
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".
