Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-25 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-25 Thread Mark Thompson
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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-25 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-25 Thread Mark Thompson
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 :
>>
>>> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-20 Thread Mark Thompson
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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-19 Thread Nicolas George
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,

-- 
  Nicolas George


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-18 Thread Mark Thompson
On 19/01/18 00:08, Carl Eugen Hoyos wrote:
> 2018-01-19 0:42 GMT+01:00 Mark Thompson :
> 
>> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH] ffmpeg: Ignore SIGPIPE

2018-01-18 Thread Carl Eugen Hoyos
2018-01-19 0:42 GMT+01:00 Mark Thompson :

> 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
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel