On Wed Jun 5 04:22:45 2024 +0000, Elizabeth Figura wrote:
The 2/6 problem is that GStreamer doesn't give us any PTS for Cinepak
videos. With decodebin_parser, the Cinepak decoder's GstVideoDecoder base class takes the DTS, and previous PTS, and fills in missing PTS; I couldn't decipher the exact formula, so I just picked something.
If PTS plus duration feels better to you, then so be it. For some
reason, the first frame does have a PTS. avidemux sets PTS on keyframes, so that's an explanation. Technically DTS is not supposed to mean PTS. They're supposed to increase at the same rate but they may not be equal. DirectShow doesn't directly have the concept of the DTS [note that SetTime() vs SetMediaTime() is not this]. GstVideoDecoder will supply PTS from DTS using this assumption, or supply PTS from the previous PTS plus duration. I'm not sure that *just* using DTS is safe either, exactly. Ideally we should just omit PTS if it isn't present. The MPEG-1 splitter doesn't always output PTS. The AVI splitter does, and if some application depends on that then I'd prefer to use AVI-specific code.
The 6/6 deadlock consists of the AVI decoder calling
IMemInputPin_Receive, which, for some reason, calls into another thread that calls qc_Notify. It works in decodebin_parser seemingly by accident; that function doesn't take any PE-side locks, it just calls into Unix code directly (and Unix-side locks are, of course, not held while calling IMemInputPin), so I just mirrored that.
If you can think of a better solution, do tell. Release the lock
before calling IMemInputPin, maybe? But the release is higher up the call stack, so there's no obvious way to do that. Release lock, call IMemInputPin, and re-acquire lock just so strmbase/pin.c can release it? I don't have my full notes unfortunately, but according to my limited notes I actually tested with a native filter [not sure which], and blocking in Notify() and calling Receive() does block, so it sounds like the deadlock you're describing is a real deadlock on Windows. What component is calling Notify() from Receive() and doing it from a separate thread? Is it a Wine component? Why is it using a separate thread if the call is apparently synchronous anyway? Perhaps the filter I tested with wasn't the AVI decoder, and the AVI decoder really is immune to this deadlock. Further speculation is that our QC logic in the AVI decoder is not what native does and should just be deleted. I won't ask you to go through testing that, but I would like to know why we're calling back from a separate thread that Receive() is blocking on anyway.
2/6: I already changed it to ignore DTS. Yeah, I checked media time a little, and it seems to just be a frame counter (at least for Cinepak AVIs).
Make it AVI specific and put it closer to the part that needs it, sure, can do. Weird that GStreamer puts that piece in the decoder, not the demuxer, but it works for them, so that's not their problem.
6/6: The offending filter is named DDX In Place, and is implemented somewhere in the game (that string shows up in strings -el st.exe). My guess would be that the game shoves everything onto the main thread; it was released in august 2000, which is around when threading started to become relevant, so I guess the devs didn't feel confident on their threading skills and just threw the bluntest solution they could find at the problem.
Googling DDX In Place (or DDX Video Renderer, another string in the exe) returns nothing relevant. Probably some third-party lib whose vendor has dropped off the internet.
I don't know what that quality control stuff is supposed to accomplish or how it's supposed to be implemented; all I know is that this call sequence does not deadlock on native. I'll add a test, if that helps. I should've done that earlier; this is indeed a strange behavior, and strange behaviors need tests.
If you don't like the new mutex, another option would be to use TryEnterCriticalSection instead, and just ignore the call if locked. This means a few of those notifications can be lost, but that's probably no big deal, they're fundamentally timing-bound anyways.