Opened 4 years ago

Last modified 21 months ago

#8917 open defect

Exit on SIGPIPE or SIGHUP - "-xerror" not working as expected

Reported by: atom Owned by:
Priority: normal Component: undetermined
Version: 4.2 Keywords: sigpipe sighup error xerror
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Expectation: ffmpeg process should die on errors and certain signals, when "-xerror" option is used.

Behaviour: Even with "-xerror", ffmpeg continues to run after receiving a SIGPIPE or SIGHUP signal.

Use cases: Using a pipe to test for "non monotonically increasing dts". In these cases, "grep" immediately (or almost immediately) matches the string "non monotonically increasing dts" and exits, closing the pipe. This should cause ffmpeg to die, but it keeps running and processing the entire input file, even with the "-xerror" option.

In the first example, this would send a SIGPIPE signal to ffmpeg, which should reasonably stop the ffmpeg process:

ffmpeg -xerror -i test.mkv -f null - 2>&1 | grep -m 1 'non monotonically increasing dts'

In the second example, the closed pipe stops "cat", and the coprocess receives a SIGHUP signal. Again, this should reasonably stop ffmpeg.

cat < <( ffmpeg -xerror -i test.mkv -f null - 2>&1 ) | grep -m 1 'non monotonically increasing dts'

If it's not reasonable that "-xerror" kills the ffmpeg process on these signals, then there should probably be one or more options to kill ffmpeg on various signals (eg an "-xsignal" option) or specific signals (eg an "-xpipe", and "-xhup" options).

As it is now, ffmpeg can consume incredible resources after it should have died. In many cases, eg processing streams and/or continuing "hidden" after a SIGHUP signal, can run indefinitely, and somewhat hidden.

nb, for duplicating this behaviour with good files, change the string that grep is matching to something such as "fps", or "mapping", or "Stream". This will match almost immediately with most/all video files, and duplicate the described behaviour.

No doubt there are countless other use cases where it would be desirable for ffmpeg to exit when receiving a SIGPIPE, SIGHUP, or other signals.

Change History (2)

comment:1 by Chris Miceli, 4 years ago

Status: newopen

Hi!

I had a look at the source to see what the current code does and a few different things were there.

SIGHUP is not handled as a signal, and as per the man page for signals (https://man7.org/linux/man-pages/man7/signal.7.html) it causes the software to terminate. A way to test this is to run

{{
ffmpeg -listen 1 -i http://127.0.0.1:9999 -c copy somefile.ogg
}}

and then kill -HUP that process. If that is not the case, I'm happy to explicitly add a signal handler to that effect as it may vary by platform. I don't speak for the ffmpeg community at large, but I suspect that it would be reasonable to be specific about that.

With regards to SIGPIPE, it is explicitly ignored because of a commit (74cf4a75f74ee3d80d021fc50b05e38bff5940e3). I will raise a question about it in the developer mailing list because it might be worth changing the behaviour.

Thanks,
Chris.

comment:2 by mldwrp, 21 months ago

Hi, I've asked the author of that commit (Mark Thompson) a question on GitHub, but got no reply for two years.
I hope I'll have better luck here — as I'm updating my higher level library at this moment — anyone is welcome to help.

@fhvwy Hello Mark,
Thank you for your work.

It so happens that this change has broken my https://github.com/costa/ffmprb as it relies on an ability to distinguish between pipe (IO) errors and other crashes.
Now, this change doesn't seem to me all that necessary, I usually follow the "fail fast" philosophy (report and expect the upper layer to handle failures) — it might benefit the end-user, but distinction between I/O failures and others is apparently important for integration.
And I believe ffmpeg is mostly used in integrations where running few processes in parallel is no problem (so each stream error can be reported separately).

There is, however, a better way than reverting this change, and it's just to report all I/O errors with a special exit code.
Thank you again.

Note: See TracTickets for help on using tickets.