[FFmpeg-devel] [PATCH 2/2] aviobuf: Avoid clearing the whole buffer in fill_buffer

[Martin Storsjö]

Normally, fill_buffer reads in one max_packet_size/IO_BUFFER_SIZE
worth of data into the buffer, slowly filling the buffer until it
is full.

Previously, when the buffer was full, fill_buffer would start over
from the start, effectively discarding all the previously buffered
data.

For files that are read linearly, the previous behaviour was fine.

For files that exhibit some amount of nonlinear read patterns,
especially mov files (where ff_configure_buffers_for_index
increases the buffer size to accomodate for the nonlinear reading!)
we would mostly be able to seek within the buffer - but whenever
we've hit the maximum buffer size, we'd discard most of the buffer
and start over with a very small buffer, so the next seek backwards
would end up outside of the buffer.

Keep one fourth of the buffered data, moving it to the start of
the buffer, freeing the rest to be refilled with future data.

For mov files with nonlinear read patterns, this almost entirely
avoids doing seeks on the lower IO level, where we previously would
end up doing seeks occasionally.

Signed-off-by: Martin Storsjö <martin at martin.st>
---
I'm open to suggestions on whether 1/4 of the buffer is a reasonable
amount to keep. It does of course incur some amount of overhead
for well behaved linear files, but is a decent improvement for
nonlinear mov files.

Alternatively we could trigger this behaviour only after we've
observed a couple seeks backwards?
---
 libavformat/aviobuf.c | 46 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 6 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 4ad734a3c3..dfc3e77016 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -534,8 +534,7 @@ static void fill_buffer(AVIOContext *s)
     FFIOContext *const ctx = (FFIOContext *)s;
     int max_buffer_size = s->max_packet_size ?
                           s->max_packet_size : IO_BUFFER_SIZE;
-    uint8_t *dst        = s->buf_end - s->buffer + max_buffer_size <= s->buffer_size ?
-                          s->buf_end : s->buffer;
+    uint8_t *dst        = s->buf_end;
     int len             = s->buffer_size - (dst - s->buffer);
 
     /* can't fill the buffer without read_packet, just set EOF if appropriate */
@@ -546,11 +545,46 @@ static void fill_buffer(AVIOContext *s)
     if (s->eof_reached)
         return;
 
-    if (s->update_checksum && dst == s->buffer) {
-        if (s->buf_end > s->checksum_ptr)
+    if (len < max_buffer_size && s->buffer_size > max_buffer_size) {
+        /* If the buffer is almost full and we're not trying to read
+           one whole buffer worth of data at once; keep some amount of
+           the currently buffered data, but move it to the start of the
+           buffer, to allow filling the buffer with more data. */
+        int keep = (s->buf_end - s->buffer)/4;
+        int shift = s->buf_end - keep - s->buffer;
+
+        if (s->update_checksum && s->checksum_ptr - s->buffer < shift) {
+            /* Checksum up to the buffer + shift position (that we're
+               shifting out of the buffer. */
             s->checksum = s->update_checksum(s->checksum, s->checksum_ptr,
-                                             s->buf_end - s->checksum_ptr);
-        s->checksum_ptr = s->buffer;
+                                             s->buffer + shift - s->checksum_ptr);
+        }
+
+        memmove(s->buffer, s->buf_end - keep, keep);
+        s->buf_end -= shift;
+        s->buf_ptr -= shift;
+        if (s->update_checksum) {
+            if (s->checksum_ptr - s->buffer < shift)
+                s->checksum_ptr = s->buffer;
+            else
+                s->checksum_ptr -= shift;
+        }
+
+        dst = s->buf_end;
+        len = s->buffer_size - (dst - s->buffer);
+    } else if (len < max_buffer_size) {
+        /* If the buffer is full so we can't fit a whole write of max_buffer_size,
+           just restart the pointers from the start of the buffer. */
+        dst = s->buffer;
+        len = s->buffer_size;
+
+        if (s->update_checksum) {
+            /* Checksum all data that gets shifted out of the buffer. */
+            if (s->buf_end > s->checksum_ptr)
+                s->checksum = s->update_checksum(s->checksum, s->checksum_ptr,
+                                                 s->buf_end - s->checksum_ptr);
+            s->checksum_ptr = s->buffer;
+        }
     }
 
     /* make buffer smaller in case it ended up large after probing */
-- 
2.37.1 (Apple Git-137.1)