On Wed Jun 19 01:57:47 2024 +0000, Elizabeth Figura wrote:
I don't think TryEnterCriticalSection() is the right solution, no. I wanted to figure out whether avidec is supposed to do any QC at all, or if v7 was the right solution, or if I could somehow find some way to prove something more interesting about native behaviour (which I unfortunately couldn't find a way to do). I did some testing with native, and while our implementation isn't correct (in particular, native does not use ICDECOMPRESS_HURRYUP), it does look like native will drop samples when Late is high enough. So we do need to keep that logic, I think. I would advocate either:
- revert to v7, with the separate lock,
- just get rid of the lock and rely on TSO guaranteed by Windows.
I don't have a preference.
That variable is 64bit, accesses are not atomic if compiled as 32bit. (And even if it was 32bit, data races are UB per the C spec; even if MS promises it's safe, I don't trust MinGW, Clang and TSan to make the same promises.)
A third option would be dropping the lock before calling IMemInput_Receive, but that's difficult with strmbase's current architecture.
A fourth option would be to access that variable with Interlocked functions, but the most convenient write operation (InterlockedExchange64) doesn't exist in Wine's headers. Wouldn't be hard to add, but that's a little out of scope for this MR, isn't it?
So I'd say using that extra lock is the best option here.