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 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. > > > > > 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".
