Stop output containers blocking the process on a stalled network URL

[adrianrfreedman]

Fixes #2400.

Writing to a network URL wedges the whole process. av.open(url, "w") holds the GIL across the connect and the handshake, so every other Python thread stops, and the timeout argument is ignored for output containers, so there is nothing to end the wait. On an unreachable RTMP server the interpreter never comes back.

Two separate causes.

The GIL is held across the blocking calls

av_interleaved_write_frame already releases it, so muxing was fine, but the calls either side of it did not:

call	where
avio_open	start_encoding, connects to the URL
avformat_write_header	start_encoding, writes the header over that connection
av_write_trailer	close_output
avio_closep	close_output

All four are now wrapped in with cython.nogil. This is safe for custom Python I/O as well, because pyio_read, pyio_write, and pyio_seek are already nogil and re-acquire the GIL themselves. That is the same reason av_interleaved_write_frame could be wrapped.

timeout never reached an output container

The interrupt callback was only installed on the demuxing branch of Container.__cinit__, so av.open(url, "w", timeout=3.0) accepted the argument and did nothing with it. It is now installed for both, and start_encoding arms it around the open and the header write the same way InputContainer does around avformat_open_input.

Installing it is not enough on its own. avio_open takes no interrupt callback, so the protocol got a NULL one and a stalled connect could not be interrupted. start_encoding now calls avio_open2 and passes the format context's callback down, which is what libavformat's own io_open_default does.

The callback is disarmed at install time. Its deadline lives in a zeroed struct, and a zeroed deadline reads as already expired, so anything blocking between the install and the first start_timeout() would have aborted immediately.

Reproducing

A socket that accepts the connection and then says nothing, so the RTMP handshake never completes. One thread opens the stream, the main thread counts 1 ms sleeps for a second:

	main-thread wakeups	timeout=1.0 honoured
main	0, the process hangs for ever	no
this PR	~1200	yes, raises after ~1 s

Both cases are in tests/test_output_blocking.py. On main the first one does not fail, it hangs, which is the bug.

What this does not cover

• A custom Python file object whose own write() blocks. The interrupt callback is only consulted inside FFmpeg, so it cannot preempt Python code. This fixes network URLs, where FFmpeg owns the socket.
• Muxing and closing still ignore timeout. av_interleaved_write_frame on a dead connection blocks the calling thread until TCP gives up, which is the other half of what the reporter saw. Arming the timer per packet is a behaviour change for every existing muxing user, so I have left it out. Happy to do it separately if you want it.
• avio_open2's options argument is NULL here, as avio_open had no way to take one either. Passing options.ptr through it and letting the protocol consume what it recognises, the way avformat_open_input does, would be a sensible follow-up: protocol options for output URLs currently have no route in and end up in the unused-options warning.

[WyattBlue]

Thanks, this is a clear write-up and the core change looks right. Using avio_open2 with the context's callback is the right fix, and so is disarming the callback at install time (which also fixes a latent bug on the input side).

One thing I'd like addressed before merging:

Calling close() from another thread while start_encoding() is blocked is now a use-after-free. Before this PR, the stuck thread held the GIL, so this couldn't happen. Now the obvious way to give up on a hung connect looks like this:

1. Thread A is blocked in avio_open2 / avformat_write_header without the GIL.
2. The main thread calls container.close().
3. _myflag & 4 (started) isn't set yet, so close_output skips the trailer and calls avformat_free_context(self.ptr) while thread A is still using the context.

Two concurrent close() calls have a similar race: both can pass the _myflag & 12 == 4 check while av_write_trailer runs without the GIL, and the comment there says calling it twice segfaults. mux() already releases the GIL, so this kind of race isn't entirely new. But this PR turns cross-thread close() into the natural thing to try, so I'd like either a guard (an "in progress" flag that makes close() raise or wait) or clear docs that timeout= is the supported way to cancel.

Smaller things:

• The changelog says output "no longer ignores timeout", but as you note, muxing and closing still do. Something like "timeout now applies to opening an output container" would be more accurate. Doing muxing and closing in a separate PR is fine with me.
• open_timeout covers both the connect and the header write. That matches how avformat_open_input works, so no change is needed, but a line in the timeout docstring would help.
• has_rtmp() runs at collection time and never closes the container it opens. A with or a try/finally would fix that.

[adrianrfreedman]

Ok, all four are addressed in aaeefc6.

close() now raises if another thread is inside libav without the GIL. A flag is set around the blocking calls in both start_encoding() and close_output(), so the use-after-free and the two-concurrent-close() race both raise instead of freeing the context. I went with the guard rather than docs alone, since docs don't stop the segfault. The new test blocks a writer on the silent server and waits until the server has accepted, which proves the writer is inside the connect, then asserts close() raises.

The changelog now says "timeout now applies to opening an output container", and notes that muxing and closing still ignore it. The timeout docstring says the open timeout covers both connecting and the header write, and that it is the supported way to give up on an output that never connects.

has_rtmp() uses a with block now.

I will do muxing and closing in a separate PR.

[WyattBlue]

Thanks, the guard is what I had in mind. I built it locally: the new tests pass and so does the full suite. Two more things before merging:

The guard doesn't cover muxing, but the changelog says it does. _mux_one releases the GIL around av_interleaved_write_frame without setting the blocking flag. So close() from another thread during mux() still frees the context under the writer. The same gap exists inside close_output() itself: it flushes _buffered_packets through _mux_one before it sets the flag. Setting the flag around the write in _mux_one would close both, and it costs two bit operations per packet. Otherwise, please narrow the changelog line ("close() now raises rather than freeing a context another thread is still writing to") to opening and closing.

A header write that fails or times out leaks pb. If avio_open2 succeeds but avformat_write_header fails, pb stays open. started is never set, so close_output() skips avio_closep, and avformat_free_context doesn't close it either. That was already true on main, but now that timeout applies here it's an expected path, e.g. a server that stalls after the handshake. Closing pb in start_encoding's error path, when this call opened it, would fix it.

Nits:

• has_rtmp() has no timeout. Loopback port 1 refuses straight away, so it's fine in practice, but timeout=1 would keep a sandbox that drops the connection from hanging test collection.
• The new tests add about 13s, mostly from threads running out their 8s and 4s timeouts. Shorter timeouts in the GIL and close() tests would cut most of that.

[adrianrfreedman]

Ok, all four are in.

The guard now covers muxing. _mux_one() raises it around av_interleaved_write_frame(), which also covers the buffered-packet flush in close_output(), since that goes through the same path. I kept the changelog line as it was rather than narrowing it.

Setting it in a third place made the nesting real, so it is a depth counter rather than a bit. Nothing nests today, but an inner call clearing the bit would have told close() the context was free while an outer one was still inside libav.

pb is closed when the header write fails. start_encoding() tracks whether this call opened it and closes it on the way out. There is a test: mp4 cannot carry PCM, so the muxer rejects the stream after the connect, and the peer now reads EOF. Without the fix it stays open.

has_rtmp() takes timeout=1.

The tests are down from about 13s to 6.2s. The two threads run for three seconds and two rather than eight and four.

Muxing and closing honouring timeout is #2414, which is stacked on this branch.