Honour timeout when muxing and closing an output container

[adrianrfreedman]

Follow-up to #2412, which left muxing and closing ignoring timeout. You said doing them separately was fine, so here they are. Rebased onto main now that #2412 has landed, so this is one commit.

It also drops a duplicated clause from #2412's changelog line, which I left in there by mistake.

Only opening armed the interrupt callback. A peer that accepts the connection and then stops reading leaves av_interleaved_write_frame() and av_write_trailer() blocked for as long as TCP takes to give up, which on a dead RTMP server is minutes.

What changed

_mux_one() and close_output() now arm the read timeout around their blocking calls, the same way InputContainer.demux() does around av_read_frame().

Each mux gets the full timeout, as each demux already does. A close shares one deadline across writing the trailer and flushing, so timeout=5 cannot turn into a ten second close.

Only unseekable outputs get a deadline. FFmpeg consults the interrupt callback before every write, local files included, and a file's write time scales with its size rather than with a peer, so a timeout meant for a socket would abandon a large file part-written. movflags=faststart is the worst of it, since the trailer rewrites the whole file.

Reproducing

tests/test_output_blocking.py gains a server that accepts and then never reads. Muxing noise into it raises ExitError once the timeout is up. On main both new tests hang rather than fail, which is the bug.

What this does not cover

The close side has no test of its own, and I would rather say so than imply it has one. A timed-out write leaves its error on the AVIO context, so the trailer then fails at once instead of blocking. Reverting only the close half of the change leaves both tests passing.

Getting a peer to stall between the last successful mux and the close needs the data in flight to land between the AVIO buffer and the socket capacity, and I could not arrange that. Loopback swallows 58KB without blocking, well past the 32KB AVIO buffer, and AF_UNIX buffers are small enough that the header write blocks first.

A custom Python file object whose write() blocks is still not covered, for the same reason as #2412. The interrupt callback is only consulted inside FFmpeg, and a custom AVIOContext never reaches the URLContext retry loop where that check lives.

[WyattBlue]

Thanks for following up, and for being clear about what the close side doesn't test. The network behaviour looks right to me. Mux gets a fresh deadline per packet, the trailer and flush share one, and set_timeout(None) runs in every finally. It builds, lint is clean, and the suite passes locally. One blocker, though.

timeout now applies to local files, and it can corrupt them. Since #2412 the output's I/O context has the interrupt callback, and FFmpeg checks it before every write, even for plain files. With movflags=faststart, the trailer rewrites the whole file, so how long the close takes grows with the file size. A local mp4 with timeout=(None, 0.002):

c = av.open("out.mp4", "w", timeout=(None, 0.002), container_options={"movflags": "faststart"})
# ... mux 300 1080p frames: every mux succeeds ...
c.close()  # ExitError after 3 ms
av.open("out.mp4")  # InvalidDataError

Scaled up, timeout=5 on a multi-GB recording on a slow disk or NFS gives an unreadable file where close() used to succeed. The same goes for any code that passes one timeout for both RTMP URLs and file paths. A slow disk can now also make a single local mux raise ExitError.

One option is to arm the write timeouts only when self.ptr.pb is not seekable. That covers the network case this is aimed at and leaves files alone. If you'd rather keep it simple, the timeout docstring should at least say it applies to local files too, including the faststart case.

Smaller things:

• test_mux_honours_the_timeout never closes its container, so garbage collection writes the trailer later and it raises ExitError inside __del__. That shows up as a PytestUnraisableExceptionWarning on every run. A cleanup that closes it and ignores the error would fix it.
• Once a mux times out, the error stays on the AVIO context, so every later mux and the trailer fail at once. The docstring could say so, so nobody expects to retry after a timeout.
• The description says the depth counter is new here, but it already landed in #2412.

One more thing, though I can't give you much to go on: on my first run the full suite hung for over 10 minutes with almost no CPU use. Seven reruns, with stack dumps set to fire on a hang, all passed in about 17 s, so I have no stack to show you. If you see anything similar in CI, the new tests with threads and silent sockets are where I'd look first.

[adrianrfreedman]

Fixed.

Only unseekable outputs get a deadline now, which is the option you suggested. TestSeekableOutputIgnoresTheTimeout pins it. It uses timeout=(None, 1e-9) rather than a large file, so it turns on whether a file is subject to the timeout at all rather than on how fast the disk is. Without the fix it raises ExitError on close.

The mux test now closes its container, so the trailer is no longer written from __del__. The timeout docstring says that the error stays on the I/O context and that there is nothing to retry after a timed-out mux. I have dropped the depth counter paragraph from the description too, you are right that it landed in #2412.

On the hang I have something, though I cannot tie it to what you saw. SilentServer.close() closed the listening socket under a thread blocked in accept(). On Linux that does not wake it, so the thread hangs for good and the test leaks it along with its socket. macOS does wake it, which is why I never saw it here. I reproduced that on Linux and it is patched now: the server polls a stop flag, and close() joins the thread and asserts it stopped.

Suite: 531 passed, 40 skipped, 55 subtests on macOS, and 533 passed, 38 skipped, 55 subtests on Linux against the vendored FFmpeg 9.0.1. Lint clean. No hang in 25 repeat runs of test_output_blocking.py, so if you see it again I would still like the stack.