Message ID | 7dc07c47-d9ba-4db3-4fd0-a5a16bbb6e22@jkqxz.net |
---|---|
State | Accepted |
Commit | 74cf4a75f74ee3d80d021fc50b05e38bff5940e3 |
Headers | show |
2018-01-19 0:42 GMT+01:00 Mark Thompson <sw@jkqxz.net>:
> To offer this suggestion in a more concrete form.
Works as expected here except for printing an error which
may be intended.
Thank you, Carl Eugen
On 19/01/18 00:08, Carl Eugen Hoyos wrote: > 2018-01-19 0:42 GMT+01:00 Mark Thompson <sw@jkqxz.net>: > >> To offer this suggestion in a more concrete form. > > Works as expected here except for printing an error which > may be intended. Yes, that's intended. Data may have been lost unexpectedly (as with any I/O error), so I believe the user should be informed. - Mark
Mark Thompson (2018-01-18): > On systems which deliver SIGPIPE (Unices), a broken pipe will currently > result in the immediate termination of the ffmpeg process (the default > disposition as required by POSIX). This is undesirable, because while > the broken pipe is likely fatal to useful cleanup of whatever component > is writing to it, there might be other components which can do useful > cleanup - for example, a muxer on another stream may still need to write > indexes to complete a file. Therefore, set the signal disposition for > SIGPIPE to ignore the signal - the call which caused the signal will > fail with EPIPE and the error will be propagated upwards like any other > I/O failure on a single stream. I think it is a bad idea to make this unconditional. But I am tired to fight uphill. Regards,
On 18/01/18 23:42, Mark Thompson wrote: > On systems which deliver SIGPIPE (Unices), a broken pipe will currently > result in the immediate termination of the ffmpeg process (the default > disposition as required by POSIX). This is undesirable, because while > the broken pipe is likely fatal to useful cleanup of whatever component > is writing to it, there might be other components which can do useful > cleanup - for example, a muxer on another stream may still need to write > indexes to complete a file. Therefore, set the signal disposition for > SIGPIPE to ignore the signal - the call which caused the signal will > fail with EPIPE and the error will be propagated upwards like any other > I/O failure on a single stream. > --- > fftools/ffmpeg.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 528849a2c6..918eb353aa 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -406,6 +406,9 @@ void term_init(void) > #ifdef SIGXCPU > signal(SIGXCPU, sigterm_handler); > #endif > +#ifdef SIGPIPE > + signal(SIGPIPE, SIG_IGN); /* Broken pipe (POSIX). */ > +#endif > #if HAVE_SETCONSOLECTRLHANDLER > SetConsoleCtrlHandler((PHANDLER_ROUTINE) CtrlHandler, TRUE); > #endif > I'll apply this tomorrow if there are no further comments. Thanks, - Mark
On 19/01/18 00:18, Mark Thompson wrote: > On 19/01/18 00:08, Carl Eugen Hoyos wrote: >> 2018-01-19 0:42 GMT+01:00 Mark Thompson <sw@jkqxz.net>: >> >>> To offer this suggestion in a more concrete form. >> >> Works as expected here except for printing an error which >> may be intended. > > Yes, that's intended. Data may have been lost unexpectedly (as with any I/O error), so I believe the user should be informed. And applied. Thanks, - Mark
Mark Thompson (2018-01-25):
> And applied.
I am glad to see that my advice was so well taken into account. From now
on, I will defer to you on matters relating to Unix system programming,
especially signals, shall I?
Regards,
On 25/01/18 23:22, Nicolas George wrote: > Mark Thompson (2018-01-25): >> And applied. > > I am glad to see that my advice was so well taken into account. From now > on, I will defer to you on matters relating to Unix system programming, > especially signals, shall I? Please don't. In this case the semantics of ignoring the signal are clear and I think we can be confident the change is ok, but that certainly isn't true in almost all other cases (especially if people are going to catch signals and do something in the signal handler). - Mark
Mark Thompson (2018-01-25): > Please don't. In this case the semantics of ignoring the signal are > clear and I think we can be confident the change is ok, but that > certainly isn't true in almost all other cases (especially if people > are going to catch signals and do something in the signal handler). Well, you pushed this patch disregarding my advice: obviously you consider your grasp on signals better than mine. Good. We need somebody that understands them. Regards,
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 528849a2c6..918eb353aa 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -406,6 +406,9 @@ void term_init(void) #ifdef SIGXCPU signal(SIGXCPU, sigterm_handler); #endif +#ifdef SIGPIPE + signal(SIGPIPE, SIG_IGN); /* Broken pipe (POSIX). */ +#endif #if HAVE_SETCONSOLECTRLHANDLER SetConsoleCtrlHandler((PHANDLER_ROUTINE) CtrlHandler, TRUE); #endif