PR #23769 opened by Niklas Haas (haasn) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23769 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23769.patch
>From 598fbea13f3dfcd37c2e5587aa75c372f923cbc4 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sat, 11 Jul 2026 12:31:54 +0200 Subject: [PATCH 1/3] avformat/shared: use strong CAS for marking PENDING This was using a weak atomic under the assumption that the `goto retry` will immediately jump back to the same line. However, that's no longer true after the introduction of the `read_block` label, which we can jump to from the error handling fallback path. In this case, we really ought to use a strong CAS to ensure that we only re-check the state if it truly changed in the meantime. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index c01d7d4e9b..ca33b7ddad 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -655,10 +655,10 @@ read_block: case BLOCK_NONE: if (s->read_only) break; /* don't mark block as pending */ - if (atomic_compare_exchange_weak_explicit(&block->state, &state, - BLOCK_PENDING, - memory_order_acquire, - memory_order_acquire)) + if (atomic_compare_exchange_strong_explicit(&block->state, &state, + BLOCK_PENDING, + memory_order_acquire, + memory_order_acquire)) { /* Acquired pending state, proceed to fetch the block */ state = BLOCK_PENDING; -- 2.52.0 >From 7acd2b9d40a776c91850a45b3fb7102c69c690ad Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sat, 11 Jul 2026 13:01:53 +0200 Subject: [PATCH 2/3] avformat/shared: give up on cache file after too many corrupt blocks If we got 10 corrupt blocks in a row, chances are, the rest of the file is completely corrupt. I didn't bother making this user-configurable as it is already sort of a niche option to work around lack of a reliable filesystem. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index ca33b7ddad..df72abff7f 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -62,6 +62,12 @@ #define HEADER_MAGIC MKTAG(u'\xFF', 'S', 'h', '$') #define HEADER_VERSION 3 +/** + * Hard watershed of consecutive failed blocks before we give up on the cache + * file altogether and assume it's entirely lost to us. + **/ +#define MAX_CORRUPT_BLOCKS 10 + static int hash_uri(uint8_t hash[HASH_SIZE], const char *uri) { struct AVHashContext *ctx = NULL; @@ -161,6 +167,7 @@ typedef struct SharedContext { uint8_t *tmp_buf; int block_size; int write_err; ///< write error occurred + int num_corrupt; /* cache file */ uint8_t *cache_data; ///< optional mmap of the cache file @@ -610,6 +617,9 @@ static int shared_read(URLContext *h, unsigned char *buf, int size) retry: switch (state) { default: + if (s->num_corrupt >= MAX_CORRUPT_BLOCKS) + goto read_block; /* assume broken cache file */ + /* We always need to read the entire block to verify integrity */ block_size = clamp_size(h, block_size, block_pos); /* filesize may have changed */ if (s->cache_data) { @@ -629,10 +639,13 @@ retry: av_log(h, AV_LOG_ERROR, "Cache corruption detected for block 0x%"PRIx64" at " "offset 0x%"PRIx64": expected CRC: 0x%08X, got: 0x%08X\n", block_id, block_pos, state, crc); - if (s->retry_corrupt) + if (s->retry_corrupt) { + s->num_corrupt++; goto read_block; + } return AVERROR(EIO); - } + } else + s->num_corrupt = 0; /* reset corrupt block count on success */ tmp += (ptrdiff_t) offset; size = FFMIN(size, block_size - offset); @@ -652,6 +665,13 @@ retry: return AVERROR(EIO); read_block: + if (s->num_corrupt == MAX_CORRUPT_BLOCKS) { + av_log(h, AV_LOG_ERROR, "Too many consecutive corrupt blocks; " + "assuming cache file is completely broken.\n"); + s->num_corrupt++; /* silence this log on subsequent reads */ + } + av_fallthrough; + case BLOCK_NONE: if (s->read_only) break; /* don't mark block as pending */ -- 2.52.0 >From 4a0b2d46055184184c36a3d2bda31eeb83523ab2 Mon Sep 17 00:00:00 2001 From: Niklas Haas <[email protected]> Date: Sat, 11 Jul 2026 13:03:24 +0200 Subject: [PATCH 3/3] avformat/shared: treat unexpected EOF as corruption This can happen if e.g. the file is truncated (e.g. by a filesystem bug or self-healing response to an underlying checksum mismatch), in which case we should treat it similar as a corruption case and retry it under -retry_corrupt. Sponsored-by: nxtedition AB Signed-off-by: Niklas Haas <[email protected]> --- libavformat/shared.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libavformat/shared.c b/libavformat/shared.c index df72abff7f..87278f1288 100644 --- a/libavformat/shared.c +++ b/libavformat/shared.c @@ -547,7 +547,7 @@ static int read_cache(SharedContext *s, uint8_t *buf, size_t size, off_t offset) while (size) { ssize_t ret = pread(s->fd, buf, size, offset); if (ret <= 0) - return ret ? AVERROR(errno) : AVERROR(EIO); + return ret ? AVERROR(errno) : AVERROR_EOF; buf += ret; offset += ret; size -= ret; @@ -630,6 +630,13 @@ retry: ret = read_cache(s, tmp, block_size, block_pos); if (ret < 0) { av_log(h, AV_LOG_ERROR, "Failed to read from cache file: %s\n", av_err2str(ret)); + if (ret == AVERROR_EOF) { /* e.g. cache appears truncated? */ + if (s->retry_corrupt) { + s->num_corrupt++; + goto read_block; + } + ret = AVERROR(EIO); /* don't propagate EOF to caller */ + } return ret; } } -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
