[FFmpeg-devel] [PATCH] avformat/cache: Separate read and seek return-values within cache_read

[Bryan Huh]

Fixes an issue where an int64_t ffurl_seek return-value was being stored
in a generic int "r" variable, leading to integer overflow when seeking
into a large file (>2GB), and ultimately a "Failed to perform internal
seek" error mesage. The "r" variable was used both for storing the
result of any seeks and for the result of any reads. Instead, I separate
the variable into an int64_t seek-variable and int read-variable.

To test, try running `ffprobe 'cache:http://<something>'` on a file that
is ~3GB large, whose moov atom is at the end of the file
---
 libavformat/cache.c |   49 +++++++++++++++++++++++++------------------------
 1 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/libavformat/cache.c b/libavformat/cache.c
index a762aa9..b654f45 100644
--- a/libavformat/cache.c
+++ b/libavformat/cache.c
@@ -156,7 +156,8 @@ static int cache_read(URLContext *h, unsigned char *buf, int size)
 {
     Context *c= h->priv_data;
     CacheEntry *entry, *next[2] = {NULL, NULL};
-    int r;
+    int64_t off;
+    int bytes_read = 0;
 
     entry = av_tree_find(c->root, &c->logical_pos, cmp, (void**)next);
 
@@ -169,21 +170,21 @@ static int cache_read(URLContext *h, unsigned char *buf, int size)
         if (in_block_pos < entry->size) {
             int64_t physical_target = entry->physical_pos + in_block_pos;
 
-            if (c->cache_pos != physical_target) {
-                r = lseek(c->fd, physical_target, SEEK_SET);
-            } else
-                r = c->cache_pos;
+            if (c->cache_pos != physical_target)
+                off = lseek(c->fd, physical_target, SEEK_SET);
+            else
+                off = c->cache_pos;
 
-            if (r >= 0) {
-                c->cache_pos = r;
-                r = read(c->fd, buf, FFMIN(size, entry->size - in_block_pos));
+            if (off >= 0) {
+                c->cache_pos = off;
+                bytes_read = read(c->fd, buf, FFMIN(size, entry->size - in_block_pos));
             }
 
-            if (r > 0) {
-                c->cache_pos += r;
-                c->logical_pos += r;
+            if (bytes_read > 0) {
+                c->cache_pos += bytes_read;
+                c->logical_pos += bytes_read;
                 c->cache_hit ++;
-                return r;
+                return bytes_read;
             }
         }
     }
@@ -191,30 +192,30 @@ static int cache_read(URLContext *h, unsigned char *buf, int size)
     // Cache miss or some kind of fault with the cache
 
     if (c->logical_pos != c->inner_pos) {
-        r = ffurl_seek(c->inner, c->logical_pos, SEEK_SET);
-        if (r<0) {
+        off = ffurl_seek(c->inner, c->logical_pos, SEEK_SET);
+        if (off<0) {
             av_log(h, AV_LOG_ERROR, "Failed to perform internal seek\n");
-            return r;
+            return off;
         }
-        c->inner_pos = r;
+        c->inner_pos = off;
     }
 
-    r = ffurl_read(c->inner, buf, size);
-    if (r == 0 && size>0) {
+    bytes_read = ffurl_read(c->inner, buf, size);
+    if (bytes_read == 0 && size>0) {
         c->is_true_eof = 1;
         av_assert0(c->end >= c->logical_pos);
     }
-    if (r<=0)
-        return r;
-    c->inner_pos += r;
+    if (bytes_read<=0)
+        return bytes_read;
+    c->inner_pos += bytes_read;
 
     c->cache_miss ++;
 
-    add_entry(h, buf, r);
-    c->logical_pos += r;
+    add_entry(h, buf, bytes_read);
+    c->logical_pos += bytes_read;
     c->end = FFMAX(c->end, c->logical_pos);
 
-    return r;
+    return bytes_read;
 }
 
 static int64_t cache_seek(URLContext *h, int64_t pos, int whence)
-- 
1.7.1