[MPlayer-dev-eng] [PATCH] Support CineForm DirectShow codec

[Reimar Döffinger]

On Tue, Feb 09, 2010 at 07:26:24PM +0100, Steinar H. Gunderson wrote:
> On Tue, Feb 09, 2010 at 07:07:26PM +0100, Reimar Döffinger wrote:
> > No, that's not the problem, I don't like that it means some loss of debugging
> > features to switch the implementation.
> 
> Which debugging features in particular do you miss?

The "unlocking unlocked", "deleting unlocked" etc. ones.
They are broken for several use-cases, not taking into account nested
lockings it seems (which actually means the whole implementation
probably is broken for that case), still...

> > While I agree with you that this kind of thing normally should be better left to
> > others, I think it's reasonable to keep the code for now, I think the necessary
> > change would be something like below.
> 
> I must admit I'm very skeptical to using two locks -- it very easily leads to
> deadlock, especilly with recursion through foreign code. (I haven't run your
> patch through Helgrind/thrcheck/tsan, but I guess it wasn't meant as a
> production-ready example.)

Hm? A lock that is immediately released without any code in-between that can block
can not cause any deadlocks, ever (though the original code may have more issues).

> What _can_ be done is to still have a single mutex, use it to protect the lock
> count/owner only (ie., don't actually hold the mutex after
> EnterCriticalSection has completed), and then if the mutex is not free, wait
> for a signal to be sent. This is pretty much the same as the semaphore
> implementation today, IIRC, and should work. It will almost certainly be
> slower and more tricky to get right than just using a recursive mutex,
> though, but I don't know if that's an issue.

Well, a semaphore is somewhat more complex, but it could be used.
Though the current semaphore code can not easily be reused it seems.