PR #25152 opened by whatisyourname0 URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25152 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25152.patch
The file protocol currently writes output without requesting storage synchronization. Flushing AVIO buffers does not change that: bytes accepted by write() may still be in the guest page cache when a VM crashes. Add a file_sync option for writable regular files: - none keeps the current behavior and remains the default. - close calls fsync() after the final buffered output, before closing the file. - write calls fsync() after each successful protocol write and again on close. Keep synchronization errors sticky so a later close cannot hide a failed write. Retry EINTR unless the interrupt callback requests cancellation. Reject unsupported configurations and non-regular destinations, validating the opened descriptor before truncation and avoiding a blocking FIFO open. The patch includes documentation and a fate-file-sync test covering synchronization timing, error propagation, cancellation and final buffered header rewrites. The test substitutes fsync only in its own executable; the production implementation has no test hook. Validation on Linux ARM64: - 303 tests passed in the minimal configured FATE build, including fate-file-sync. This was not the full codec/sample matrix. - 11 CLI error/rejection cases and 6 direct AVIO error cases passed. An fsync-disabled build correctly rejects the option. - The new test detects deliberate regressions that omit fsync, discard a sticky error or overwrite errno before preserving it. - An initial four VM crash/reboot trials preserved completed close-mode output and acknowledged write-mode checkpoints. - A subsequent fsync/fdatasync comparison passed 12 VM crash/reboot trials: six per method, covering file extension, an existing WAV header rewrite, and extension followed by header finalization. All 78 cumulative length/hash checks passed. The probes stopped after the final write synchronization and before close, so an extra close-time sync could not mask a write-time failure. The option remains opt-in because frequent synchronization can cause input backpressure. In an exploratory host-mounted output test, enabling write reduced throughput from 373 to 221 MiB/s with normal buffering and from 159 to 74 MiB/s with flush_packets=1. A separate interleaved comparison found no clear slowdown with the default policy: original/patched medians were 451/454 MiB/s and 158/159 MiB/s respectively, within the observed variation. I also compared fsync and fdatasync using identical AVIO writes and synchronization counts on an ext4 filesystem backed by a VZ virtio-blk disk. After two warmups, six measured repetitions per method gave these median elapsed times: | Operation | fsync | fdatasync | | --- | ---: | ---: | | Append 4 MiB in 8 KiB writes | 5.236 s | 5.208 s | | Rewrite an existing WAV header | 10.09 ms | 5.28 ms | | Append 4 MiB and finalize the WAV header | 5.099 s | 5.261 s | fdatasync helped the header-only case, but showed no consistent benefit for growing files. Both methods preserved the tested data, but fdatasync does not promise the same persistence of all file metadata. This patch therefore retains fsync; it does not introduce fdatasync or batching. These tests established durable output names and parent directories before recording. The patch does not persist new names or subsequent renames, recover arbitrary unfinished media containers, or protect bytes still in input/AVIO buffers. VM process failure was tested; host power loss was not. Directory synchronization and application integration are separate follow-ups. The measurements are local exploratory results, not cross-platform performance or durability guarantees. Is the file protocol the right place for this policy, and does the none/close/write interface look appropriate? >From 40460d9baea012cb202b11d18bbdbe074c03af20 Mon Sep 17 00:00:00 2001 From: whatisyourname0 <[email protected]> Date: Sat, 17 Oct 2026 12:37:54 +0000 Subject: [PATCH] avformat/file: add opt-in output synchronization Add none, close and write synchronization policies for regular-file output. Preserve synchronization errors through close and cover buffering, retries and cancellation with a FATE regression test. --- Changelog | 1 + configure | 2 + doc/protocols.texi | 18 ++++ libavformat/Makefile | 1 + libavformat/file.c | 101 +++++++++++++++++- libavformat/tests/file_sync.c | 194 ++++++++++++++++++++++++++++++++++ tests/fate/libavformat.mak | 5 + 7 files changed, 321 insertions(+), 1 deletion(-) create mode 100644 libavformat/tests/file_sync.c diff --git a/Changelog b/Changelog index 32e7c8bfe7..c3fe5dc294 100644 --- a/Changelog +++ b/Changelog @@ -2,6 +2,7 @@ Entries are sorted chronologically from oldest to youngest within each release, releases are sorted from youngest to oldest. version <next>: +- optional regular-file output synchronization (file_sync) - ASTC image encoding and decoding via libastcenc, with .astc and KTX 1.0 muxers and demuxers - extensively improved AAC encoder - APV Vulkan encoder diff --git a/configure b/configure index c97ddb186f..a7d00db1b5 100755 --- a/configure +++ b/configure @@ -2506,6 +2506,7 @@ SYSTEM_FUNCS=" CommandLineToArgvW elf_aux_info fcntl + fsync getaddrinfo getauxval getenv @@ -6881,6 +6882,7 @@ check_func ${malloc_prefix}posix_memalign && enable posix_memalign check_func access check_func_headers stdlib.h arc4random_buf check_lib clock_gettime time.h clock_gettime || check_lib clock_gettime time.h clock_gettime -lrt +check_func_headers unistd.h fsync check_func fcntl check_func fork check_func gethrtime diff --git a/doc/protocols.texi b/doc/protocols.texi index aa3b88fd58..1ef8fefbc0 100644 --- a/doc/protocols.texi +++ b/doc/protocols.texi @@ -330,6 +330,24 @@ ffmpeg -i file:input.mpeg output.mpeg This protocol accepts the following options: @table @option +@item file_sync +Request explicit storage synchronization for regular-file output. +The default @code{none} preserves buffered file I/O behavior. +@code{close} calls @code{fsync()} before closing the output. +@code{write} calls @code{fsync()} after each successful low-level write and +also before closing the output. +Synchronization failures are returned as output errors. +This option requires platform support for @code{fsync()} and regular files; +unsupported configurations fail rather than silently disabling synchronization. + +This does not flush pending AVIO buffers by itself; use @code{flush_packets} +when lower buffering latency is needed. +Synchronization can block and reduce throughput. +It does not persist directory entries or subsequent renames, guarantee +media-container recovery, or protect bytes still in input or AVIO buffers. +The guarantee is that of the platform's @code{fsync()}, not a universal +physical power-loss guarantee. + @item truncate Truncate existing files on write, if set to 1. A value of 0 prevents truncating. Default value is 1. diff --git a/libavformat/Makefile b/libavformat/Makefile index b707193eec..952c07cb44 100644 --- a/libavformat/Makefile +++ b/libavformat/Makefile @@ -799,6 +799,7 @@ TESTPROGS-$(CONFIG_RTPDEC) += rtpdec FIFO-MUXER-TESTPROGS-$(CONFIG_NETWORK) += fifo_muxer TESTPROGS-$(CONFIG_FIFO_MUXER) += $(FIFO-MUXER-TESTPROGS-yes) TESTPROGS-$(CONFIG_FFRTMPCRYPT_PROTOCOL) += rtmpdh +TESTPROGS-$(CONFIG_FILE_PROTOCOL) += file_sync TESTPROGS-$(CONFIG_HTTP_PROTOCOL) += http TESTPROGS-$(CONFIG_NETWORK) += noproxy TESTPROGS-$(CONFIG_SRTP) += srtp diff --git a/libavformat/file.c b/libavformat/file.c index bb99198a6a..04daaa6530 100644 --- a/libavformat/file.c +++ b/libavformat/file.c @@ -98,6 +98,8 @@ typedef struct FileContext { int trunc; int blocksize; int pkt_size; + int file_sync; /**< Requested regular-file synchronization policy. */ + int sync_error; /**< First synchronization error, retained until close. */ int follow; int seekable; #if HAVE_DIRENT_H @@ -108,7 +110,17 @@ typedef struct FileContext { #endif } FileContext; +enum FileSyncMode { + FILE_SYNC_NONE, + FILE_SYNC_CLOSE, + FILE_SYNC_WRITE, +}; + static const AVOption file_options[] = { + { "file_sync", "synchronize regular-file output", offsetof(FileContext, file_sync), AV_OPT_TYPE_INT, { .i64 = FILE_SYNC_NONE }, FILE_SYNC_NONE, FILE_SYNC_WRITE, AV_OPT_FLAG_ENCODING_PARAM, .unit = "file_sync" }, + { "none", "no explicit storage synchronization", 0, AV_OPT_TYPE_CONST, { .i64 = FILE_SYNC_NONE }, 0, 0, AV_OPT_FLAG_ENCODING_PARAM, .unit = "file_sync" }, + { "close", "synchronize before closing output", 0, AV_OPT_TYPE_CONST, { .i64 = FILE_SYNC_CLOSE }, 0, 0, AV_OPT_FLAG_ENCODING_PARAM, .unit = "file_sync" }, + { "write", "synchronize each output write", 0, AV_OPT_TYPE_CONST, { .i64 = FILE_SYNC_WRITE }, 0, 0, AV_OPT_FLAG_ENCODING_PARAM, .unit = "file_sync" }, { "truncate", "truncate existing files on write", offsetof(FileContext, trunc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, AV_OPT_FLAG_ENCODING_PARAM }, { "blocksize", "set I/O operation maximum block size", offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM }, { "follow", "Follow a file as it is being written", offsetof(FileContext, follow), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, AV_OPT_FLAG_DECODING_PARAM }, @@ -157,12 +169,50 @@ static int file_read(URLContext *h, unsigned char *buf, int size) return (ret == -1) ? AVERROR(errno) : ret; } +/** Synchronize output, retry interruptions, and retain the first failure. */ +static int file_sync_output(URLContext *h) +{ + FileContext *c = h->priv_data; + int ret; + + if (c->sync_error) + return c->sync_error; +#if HAVE_FSYNC + for (;;) { + if (fsync(c->fd) == 0) { + ret = 0; + break; + } + ret = AVERROR(errno); + if (ret != AVERROR(EINTR)) + break; + if (ff_check_interrupt(&h->interrupt_callback)) { + ret = AVERROR_EXIT; + break; + } + } +#else + ret = AVERROR(ENOSYS); +#endif + if (ret < 0) + c->sync_error = ret; + return ret; +} + static int file_write(URLContext *h, const unsigned char *buf, int size) { FileContext *c = h->priv_data; int ret; size = FFMIN(size, c->blocksize); + if (c->sync_error) + return c->sync_error; ret = write(c->fd, buf, size); + if (ret > 0 && c->file_sync == FILE_SYNC_WRITE) { + int err = file_sync_output(h); + + if (err < 0) + return err; + } return (ret == -1) ? AVERROR(errno) : ret; } @@ -238,8 +288,12 @@ static int fd_check(URLContext *h, int mask) static int file_close(URLContext *h) { FileContext *c = h->priv_data; + int err = c->file_sync ? file_sync_output(h) : 0; int ret = close(c->fd); - return (ret == -1) ? AVERROR(errno) : 0; + + if (err < 0) + return err; + return ret == -1 ? AVERROR(errno) : 0; } /* XXX: use llseek */ @@ -309,6 +363,14 @@ static int file_open(URLContext *h, const char *filename, int flags) av_strstart(filename, "file:", &filename); + if (c->file_sync) { +#if !HAVE_FSYNC || !HAVE_FCNTL || !defined(O_NONBLOCK) + return AVERROR(ENOSYS); +#endif + if (!(flags & AVIO_FLAG_WRITE)) + return AVERROR(EINVAL); + } + if (flags & AVIO_FLAG_WRITE && flags & AVIO_FLAG_READ) { access = O_CREAT | O_RDWR; if (c->trunc) @@ -323,9 +385,46 @@ static int file_open(URLContext *h, const char *filename, int flags) #ifdef O_BINARY access |= O_BINARY; #endif + if (c->file_sync) { + /* Check the opened object before truncating it; do not block on FIFOs. */ + access &= ~O_TRUNC; +#ifdef O_NONBLOCK + access |= O_NONBLOCK; +#endif + } fd = avpriv_open(filename, access, 0666); if (fd == -1) return AVERROR(errno); + if (c->file_sync) { +#if HAVE_FSYNC && HAVE_FCNTL && defined(O_NONBLOCK) + int err; + + if (fstat(fd, &st) < 0) { + err = AVERROR(errno); + goto sync_open_fail; + } + if (!S_ISREG(st.st_mode)) { + err = AVERROR(EINVAL); + goto sync_open_fail; + } + err = fcntl(fd, F_GETFL); + if (err < 0 || fcntl(fd, F_SETFL, err & ~O_NONBLOCK) < 0) { + err = AVERROR(errno); + goto sync_open_fail; + } + if (c->trunc && ftruncate(fd, 0) < 0) { + err = AVERROR(errno); + goto sync_open_fail; + } + goto sync_open_done; +sync_open_fail: + close(fd); + return err; +#endif + } +#if HAVE_FSYNC && HAVE_FCNTL && defined(O_NONBLOCK) +sync_open_done: +#endif c->fd = fd; h->is_streamed = !fstat(fd, &st) && S_ISFIFO(st.st_mode); diff --git a/libavformat/tests/file_sync.c b/libavformat/tests/file_sync.c new file mode 100644 index 0000000000..9d522dbb57 --- /dev/null +++ b/libavformat/tests/file_sync.c @@ -0,0 +1,194 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "config.h" + +#include <errno.h> +#include <stdio.h> +#if HAVE_UNISTD_H +#include <unistd.h> +#endif + +static int sync_calls, sync_errno, interrupt_calls, cancel_sync; + +#if HAVE_FSYNC +/** Count synchronization attempts and inject a single syscall failure. */ +static int test_fsync(int fd) +{ + sync_calls++; + if (sync_errno) { + errno = sync_errno; + sync_errno = 0; + return -1; + } + return fsync(fd); +} +#endif + +/* Like fifo_muxer, replace the protocol object in the statically linked test. + * Only this translation unit substitutes fsync; production code has no hook. */ +#define fsync test_fsync +#include "libavformat/file.c" +#undef fsync + +#include "libavutil/dict.h" + +/** Exercise callbacks that alter errno, with optional cancellation. */ +static int interrupt(void *opaque) +{ + interrupt_calls++; + errno = ENOENT; + return cancel_sync && sync_calls > 0; +} + +#define CHECK(expr) do { \ + if (!(expr)) { \ + fprintf(stderr, "line %d: %s\n", __LINE__, #expr); \ + ret = 1; \ + goto end; \ + } \ +} while (0) + +/** Check AVIO buffering, error propagation, and persistent file contents. */ +static av_unused int check_output(const char *path, const char *mode, + int fault, int cancel) +{ + AVIOContext *pb = NULL; + AVDictionary *opts = NULL; + AVIOInterruptCB cb = { interrupt, NULL }; + const unsigned char data[] = { 0, 13, 10, 26, 255, 42 }; + unsigned char actual[sizeof(data)]; + FILE *f = NULL; + int ret = 0, result, expected = 0, expected_calls; + int writing = !strcmp(mode, "write"); + int enabled = strcmp(mode, "none") != 0; + + sync_calls = sync_errno = interrupt_calls = cancel_sync = 0; + av_dict_set(&opts, "file_sync", mode, 0); + result = avio_open2(&pb, path, AVIO_FLAG_WRITE, &cb, &opts); + CHECK(result >= 0); + CHECK(!av_dict_count(opts)); + sync_errno = fault; + cancel_sync = cancel; + if (enabled && fault && fault != EINTR) + expected = AVERROR(fault); + else if (enabled && fault == EINTR && cancel) + expected = AVERROR_EXIT; + + avio_write(pb, data, sizeof(data)); + CHECK(sync_calls == 0); /* Still in AVIO, not durable yet. */ + avio_flush(pb); + CHECK(pb->error == (writing ? expected : 0)); + CHECK(sync_calls == (writing ? 1 + (fault == EINTR && !cancel) : 0)); + if (writing && expected) { + int calls = sync_calls; + + avio_write(pb, data, sizeof(data)); + avio_flush(pb); + CHECK(pb->error == expected); + CHECK(sync_calls == calls); + } + result = avio_closep(&pb); + CHECK(result == expected); + CHECK(!pb); + expected_calls = enabled ? 1 + (fault == EINTR && !cancel) : 0; + if (writing && !expected) + expected_calls++; + CHECK(sync_calls == expected_calls); + CHECK(interrupt_calls >= (enabled && fault == EINTR)); + f = fopen(path, "rb"); + CHECK(f); + CHECK(fread(actual, 1, sizeof(actual), f) == sizeof(actual)); + CHECK(!memcmp(data, actual, sizeof(data))); + CHECK(fgetc(f) == EOF); +end: + if (f) + fclose(f); + avio_closep(&pb); + av_dict_free(&opts); + return ret; +} + +/** Verify the final buffered header rewrite is synchronized on close. */ +static av_unused int check_tail(const char *path) +{ + AVIOContext *pb = NULL; + AVDictionary *opts = NULL; + FILE *f = NULL; + char actual[7]; + int ret = 0; + + sync_calls = sync_errno = cancel_sync = 0; + av_dict_set(&opts, "file_sync", "write", 0); + CHECK(avio_open2(&pb, path, AVIO_FLAG_WRITE, NULL, &opts) >= 0); + avio_write(pb, (const unsigned char *)"header", 6); + avio_flush(pb); + CHECK(sync_calls == 1); + CHECK(avio_seek(pb, 0, SEEK_SET) == 0); + avio_write(pb, (const unsigned char *)"HEADER!", 7); + CHECK(avio_closep(&pb) == 0); + CHECK(sync_calls == 3); /* Final buffered rewrite and close. */ + f = fopen(path, "rb"); + CHECK(f); + CHECK(fread(actual, 1, sizeof(actual), f) == sizeof(actual)); + CHECK(!memcmp(actual, "HEADER!", sizeof(actual))); + CHECK(fgetc(f) == EOF); +end: + if (f) + fclose(f); + avio_closep(&pb); + av_dict_free(&opts); + return ret; +} + +int main(void) +{ + static const char *const modes[] av_unused = { "none", "close", "write" }; + static const int faults[] av_unused = { 0, EIO, ENOSPC, EINTR }; + char *path = NULL; + AVDictionary *opts = NULL; + AVIOContext *pb = NULL; + int fd, ret = 0, result; + + fd = avpriv_tempfile("ff-file-sync-", &path, 0, NULL); + CHECK(fd >= 0); + close(fd); +#if HAVE_FSYNC && HAVE_FCNTL && defined(O_NONBLOCK) + for (int i = 0; i < FF_ARRAY_ELEMS(modes); i++) { + for (int j = 0; j < FF_ARRAY_ELEMS(faults); j++) + CHECK(!check_output(path, modes[i], faults[j], 0)); + if (i) + CHECK(!check_output(path, modes[i], EINTR, 1)); + } + CHECK(!check_tail(path)); + av_dict_set(&opts, "file_sync", "write", 0); + result = avio_open2(&pb, path, AVIO_FLAG_READ, NULL, &opts); + CHECK(result == AVERROR(EINVAL)); +#else + av_dict_set(&opts, "file_sync", "write", 0); + result = avio_open2(&pb, path, AVIO_FLAG_WRITE, NULL, &opts); + CHECK(result == AVERROR(ENOSYS)); +#endif +end: + avio_closep(&pb); + av_dict_free(&opts); + if (path) + remove(path); + av_free(path); + return ret; +} diff --git a/tests/fate/libavformat.mak b/tests/fate/libavformat.mak index 45878891fd..f0f93b795c 100644 --- a/tests/fate/libavformat.mak +++ b/tests/fate/libavformat.mak @@ -1,3 +1,8 @@ +FATE_LIBAVFORMAT-$(CONFIG_FILE_PROTOCOL) += fate-file-sync +fate-file-sync: libavformat/tests/file_sync$(EXESUF) +fate-file-sync: CMD = run libavformat/tests/file_sync$(EXESUF) +fate-file-sync: CMP = null + #FATE_LIBAVFORMAT-$(HAVE_PTHREADS) += fate-async #fate-async: libavformat/tests/async$(EXESUF) #fate-async: CMD = run libavformat/tests/async -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
