[MPlayer-dev-eng] Failing to exit on SIGINT/SIGHUP - Cache2.c cache_read

[Reimar Döffinger]

On Wed, Feb 17, 2010 at 09:20:21PM +0000, Lee Jackson wrote:
> The PROBLEM I have is that sending a sigint/sighup/sigterm to the process, @ 50% of the time will end up with the mencoder process caught in an endless loop within the stream/cache2.c cache_read function since s->eof never seems to evaluate as true (exit(0) is called but the cache_read loop doesn't end).

Huh? I think you are bit confused, after exit() is called the process is done, complete, finished, ...
Are you killing the cache process instead of the main mencoder process?

> Hence the process never exits and it causes all sorts of problems for my framework (and as an aside, if anyone can suggest another/better way to take a single video source and transcode it to multiple formats I would be very interested).

using MPlayer with -vo yuv4mpeg -ao pcm each into its own pie, and cat + tee
to duplicate the data into multiple more pipes?
Has a chance to be slower though with high resolution input due to insufficient buffering.

> Now to fix this I added additional signal handlers e.g. signal(SIGINT,exit_sighandler) , added a simple flag to the exit_sighandler e.g.
> 
> static void exit_sighandler(int x){
>   bTerminated_Flag = 1;
> }

Please not a fourth variable for the same thing.
at_eof, interrupted and async_quit_request already exist.

> and changed the lines (112-114 in the current SVN version) to :
>   // eof?
>   if(s->eof) break;
>   if (bTerminated_Flag != 0) break;
> 
> And at last, mencoder would reliably exit while transcoding!
> 
> Now I know this is a hack but I didnt have the time then to get to grips with the cache handling code and work out why s->eof wasnt being set to true when the app was supposed to exit OR how to set it to true without using the cludge of a seperate flag (I dont really have the time now).
> 
> I also have no idea of the consequences of this change so I've been hoping someone else would come along and fix it. 10 months later, it isnt fixed so can anyone help me to put together a patch to fix this or suggest a better fix than this?

Side effects:
1) you should set eof, otherwise it might not actually work that well at all
2) not all parts will handle a failing read well, you might end up e.g. with a
   corrupted last frame, which is why I think aborting at this point should at
   most be done after the second signal.

Better fixes:
1) don't kill the cache process!
2) install a signal handler for the cache process and make it set eof on exit.
3) a variant of 1), get rid of the separate cache process by changing
   #undef PTHREAD_CACHE
   to
   #define PTHREAD_CACHE 1
   inr config.h
4) basically what you did, but a) only after the signal handler was called
   multiple times, and setting eof
5) sending SIGKILL to any remaining processes, and/or add a "panic" case
   to hard-exit after several kills (might produce broken files).