[x264-devel] Updated (and hopefully final) threaded slicetype patch

[David DeHaven]

> I have instrumented the code a bit to quickly see which paths are
> taken, see below
>
> I could get it deadlocked, this are the last two prints:
>
> ../x264/x264/x264 -v --b-adapt 2 --bframes 4 -B 5000 --threads 2 -o
> test.x264 test_1280x720.yuv
>
> ...
> frame_list_put() was not full
> frame_list_put() was not full
> frame_list_get() 24, 0: 135788592
> frame_list_get() has place, signaling space.
> frame_list_put() full, waiting for space.
>
>
> I think the x264_pthread_cond_broadcast( &slist->cv_empty ) must be
> within the mutex lock

No, it does not need to be (and may not behave properly if the mutex  
is still locked when the broadcast is done).

I use pthread_cond_signal instead and it's always done outside the  
critical section. I have zero thread sync problems with that code:
pthread_mutex_lock(&lock);
[critical section]
pthread_mutex_unlock(&lLock);
pthread_cond_signal(&lockSignal);

This code is in use on Mac OS X and Linux. It's also a case where the  
condition is only in use by two threads, so a signal is more  
appropriate than a broadcast.

I can see one potential problem since you don't seem to be limited to  
two threads, it's not spinning on the conditional that  
pthread_cond_wait is waiting on. So if you have say two threads that  
end up blocking because the list is full, when pthread_cond_broadcast  
is called BOTH threads will unblock and assume that it's safe to add  
the frame which may not be the case. If pthread_cond_signal were used  
instead, it might be safe since it only unblocks one thread, but I  
would still re-check the condition before continuing anyways.

-DrD-