[MPlayer-dev-eng] [PATCH] use pthreads for caching

[Reimar Döffinger]

Hello,
On Fri, Jan 26, 2007 at 11:42:14PM +0100, A Mennucc wrote:
> Summary: to cache data, the code stream/cache2.c forks a subprocess
> (on all archs but win32); while trying to debug some Debian bugs, I
> have come to the conclusion that this does create some of the problems
> I was seeing. So I attach a patch that uses pthreads instead.  Please
> test it (I did not test the code for Windows against regressions).

Please split the patch in separate parts.
IMO e.g. the
> -#ifndef WIN32
> +#ifdef CACHE_FORKS

change is a good candidate to be applied first since I think it makes
the code easier to understand anyway.
I also think you can further reduce the size of the patch if you first
change all #ifndef WIN32 to that, you a bit of the patch is just
reordering code.

> My impression is this: after the fork, there are two signal handler
> around; then there is a bug in the codec (or in the gmplayer GUI) that
> triggers a signal from the Xlibs; but it gets caught in the wrong
> signal handler; from there on , mayhem.

Certainly not, though it can happen that only one of the mplayer
instances gets killed.

> @@ -296,18 +341,24 @@
>  	if(s->eof) break; // file is smaller than prefill size
>  	if(mp_input_check_interrupt(PREFILL_SLEEP_TIME))
>  	  return 0;
> +	usec_sleep(100); // no use cycling like crazy while waiting 

Unrelated and actually wrong. If mp_input_check_interrupt does not sleep
at least PREFILL_SLEEP_TIME there is a bug somewhere else.

> -  if(!s->cache_pid) return stream_fill_buffer(s);
> +  if(!CACHE_HAS_SUBPROCESS(s)) return stream_fill_buffer(s);

IMO just leave that as it is, with cache_pid always == 0 if forking
isn't used.

Greetings,
Reimar Döffinger