PR #23434 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23434 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23434.patch
>From 83bbeea065b8feee02bdfe79c9054760dde3716e Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 13:42:22 +0200 Subject: [PATCH 1/8] avformat/shared: error out if filesize does not match expected If this happens, something is almost surely wrong with the cache file (e.g. mismatched source file), so it's much better to error out rather than hit silent data corruption. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index 0b02dd42be..0d7bd0555b 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -292,8 +292,11 @@ static int shared_open(URLContext *h, const char *arg, int flags, AVDictionary * if (filesize < 0 && filesize != AVERROR(ENOSYS)) { ret = (int) filesize; goto fail; - } else if (filesize > 0) - set_filesize(h, filesize); + } else if (filesize > 0) { + ret = set_filesize(h, filesize); + if (ret < 0) + goto fail; + } } if (filesize > 0) { @@ -803,8 +806,10 @@ static int64_t shared_seek(URLContext *h, int64_t pos, int whence) if (filesize) return filesize; res = ffurl_seek(s->inner, pos, whence); - if (res > 0) - set_filesize(h, res); + if (res > 0) { + if (set_filesize(h, res) < 0) + return AVERROR(EINVAL); + } return res; case SEEK_SET: break; @@ -820,7 +825,9 @@ static int64_t shared_seek(URLContext *h, int64_t pos, int whence) res = ffurl_seek(s->inner, pos, whence); if (res < 0) return res; - set_filesize(h, res - pos); /* Opportunistically update known filesize */ + /* Opportunistically update known filesize */ + if (set_filesize(h, res - pos) < 0) + return AVERROR(EINVAL); av_log(h, AV_LOG_DEBUG, "Inner seek to 0x%"PRIx64"\n", res); return s->pos = s->inner_pos = res; default: -- 2.52.0 >From b97ff62529a4232175bb8273d56052892ff9a8d3 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 13:47:04 +0200 Subject: [PATCH 2/8] avformat/shared: set default cache timeout to 10 ms This value is matched to the typical seek latency in a reasonably capable 7200 rpm disk device, as well as the typical latency of an on-premise HTTP request. Note that this change should rarely have a significant effect, because it only matters when using multiple concurrent processes, and one process is somehow stuck in I/O (or died). Since we sleep in a loop for 1/16th of the requested timeout value, this should only increase the effective read latency by up to ~500 us on top of the actual underlying latency. The alternative is hammering the same underlying resource with the exact same requests at the exact same time (e.g. during init). Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- doc/protocols.texi | 2 +- libavformat/shared.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/protocols.texi b/doc/protocols.texi index a5ebd98a65..0ac28e78b5 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -1613,7 +1613,7 @@ and cache the same block at the same time. If this timeout elapses, it's assumed that the other process may have gotten stuck or died in the meantime. If set to zero, no waiting is done and all processes will immediately race -to try and fetch the same missing blocks themselves. Defaults to 0. +to try and fetch the same missing blocks themselves. Defaults to 10000 (10 ms). @item retry_errors If true (the default), transient read errors from the underlying input stream diff --git a/libavformat/shared.c b/libavformat/shared.c index 0d7bd0555b..ad6279d077 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -864,7 +864,7 @@ static const AVOption options[] = { { "block_shift", "Set the base 2 logarithm of the block size", OFFSET(block_shift), AV_OPT_TYPE_INT, {.i64 = 15}, 9, 30, .flags = D }, { "read_only", "Don't write data to the cache, only read from it", OFFSET(read_only), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, .flags = D }, { "cache_verify", "Verify correctness of the cache against the source", OFFSET(verify), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, .flags = D }, - { "cache_timeout", "Time in us to wait before re-fetching pending blocks", OFFSET(timeout), AV_OPT_TYPE_INT64, {.i64 = 0}, 0, INT64_MAX, .flags = D }, + { "cache_timeout", "Time in us to wait before re-fetching pending blocks", OFFSET(timeout), AV_OPT_TYPE_INT64, {.i64 = 10000}, 0, INT64_MAX, .flags = D }, { "retry_errors", "Re-request blocks even if they previously failed", OFFSET(retry_errors), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, .flags = D }, {0}, }; -- 2.52.0 >From 872f03b17be13e55fcdf7129f9cd003318661496 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:00:20 +0200 Subject: [PATCH 3/8] avformat/shared: don't read directly into cache file when racing writes Instead, read to the output/temporary buffer (write_back path). This is to lessen the impact of racing the write against other clients trying to race the same pending block. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index ad6279d077..79e3399915 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -721,7 +721,7 @@ retry: } int write_back = 1; - if (s->cache_data) { + if (s->cache_data && !pending_since) { /* Read directly into memory mapped cache file */ tmp = s->cache_data + block_pos; write_back = 0; -- 2.52.0 >From b0e2c9ec9b7bfe02e71714ff071e46dfe4be1473 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:16:12 +0200 Subject: [PATCH 4/8] avformat/shared: add missing ret = 0 Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libavformat/shared.c b/libavformat/shared.c index 79e3399915..20e4379f14 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -326,6 +326,7 @@ static int shared_open(URLContext *h, const char *arg, int flags, AVDictionary * h->max_packet_size = s->block_size; h->min_packet_size = s->block_size; + ret = 0; fail: if (ret < 0) -- 2.52.0 >From 5fbef18e1d3dfaaf6ea3084d4eafd57843ed255c Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:20:51 +0200 Subject: [PATCH 5/8] avformat/shared: use flock() instead of fcntl() flock() also locks against accesses by other threads of the same process, unlike fcntl(). Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index 20e4379f14..7375a360d7 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -373,8 +373,7 @@ static int cache_map(URLContext *h, int64_t filesize) static int spacemap_remap(URLContext *h, size_t map_size) { SharedContext *s = h->priv_data; - struct flock fl = { .l_type = F_WRLCK }; - int ret, did_grow = 0; + int ret, did_grow = 0, locked = 0; if (map_size <= s->map_size) return 0; @@ -390,12 +389,12 @@ static int spacemap_remap(URLContext *h, size_t map_size) goto skip_resize; /* Lock the spacemap to ensure nobody else is currently resizing it */ - ret = fcntl(s->mapfd, F_SETLKW, &fl); + ret = flock(s->mapfd, LOCK_EX); if (ret < 0) { ret = AVERROR(errno); goto fail; } - fl.l_type = F_UNLCK; + locked = 1; /* Refresh filesize after acquiring the lock */ ret = fstat(s->mapfd, &st); @@ -427,15 +426,16 @@ skip_resize: goto fail; } - /* fl.l_type is set to F_UNLCK only after successful lock */ - if (fl.l_type == F_UNLCK) - fcntl(s->mapfd, F_SETLK, &fl); + if (locked) { + flock(s->mapfd, LOCK_UN); + locked = 0; + } return did_grow; fail: - if (fl.l_type == F_UNLCK) - fcntl(s->mapfd, F_SETLK, &fl); + if (locked) + fcntl(s->mapfd, LOCK_UN); av_log(h, AV_LOG_ERROR, "Failed to resize space map: %s\n", av_err2str(ret)); return ret; } -- 2.52.0 >From 5fa29d71ce759f1a1e2ce44fef90797668f10cd6 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:23:06 +0200 Subject: [PATCH 6/8] avformat/shared: robustness fix for seek-past-end If the filesize is known as a result of AVERROR_EOF on a block that ends before the current seek position, this might end up negative. Error out cleanly instead of aborting. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index 7375a360d7..ead3fdeec9 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -789,7 +789,8 @@ retry: } size = FFMIN(bytes_read - offset, size); - av_assert0(size > 0); + if (size <= 0) + return AVERROR_EOF; if (tmp != buf) memcpy(buf, &tmp[offset], size); s->pos += size; -- 2.52.0 >From 001120a66c5a42f43ad2d9a023028d131ef5ed47 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:25:15 +0200 Subject: [PATCH 7/8] avformat/shared: allow AVERROR_EXIT/EAGAIN as transient failures These shouldn't really permanently invalidate the block. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index ead3fdeec9..ecd31a21da 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -744,11 +744,15 @@ retry: else if (ret < 0) { av_log(h, AV_LOG_ERROR, "Failed to read block 0x%"PRIx64": %s\n", block_id, av_err2str(ret)); + int new_state = BLOCK_FAILED; + if (ret == AVERROR(EAGAIN) || ret == AVERROR_EXIT) + new_state = BLOCK_NONE; /* transient error, allow retries */ + /* Try to mark block as failed; ignore errors - any mismatch * here will mean that either another thread already marked it * as failed, or successfully cached it in the meantime */ atomic_compare_exchange_strong_explicit(&block->state, &state, - BLOCK_FAILED, + new_state, memory_order_relaxed, memory_order_relaxed); return ret; -- 2.52.0 >From cd85d6105faf4dab5e7721d3a783602c8aea3aaa Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Wed, 10 Jun 2026 14:34:21 +0200 Subject: [PATCH 8/8] avformat/shared: propagate correct short seek size Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index ecd31a21da..3f3b3a5e0d 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -857,9 +857,7 @@ static int shared_get_short_seek(URLContext *h) { SharedContext *s = h->priv_data; int ret = ffurl_get_short_seek(s->inner); - if (ret < 0) - return ret; - return FFMAX(ret, s->block_size); + return ret > 0 ? FFMAX(ret, s->block_size) : s->block_size; } #define OFFSET(x) offsetof(SharedContext, x) -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
