On Tue Aug 30 11:35:12 2022 +0000, Nikolay Sivov wrote:
Separating current command looks okay to me, but stopping the session on CMD_END is causing problems by sending MESessionStopped(MF_E_INVALIDREQUEST). You can see this with attached mfplay program, that's using EVR internally. My original idea for CMD_END was to use it as a marker in commands queue that would block pending commands, allowing for end-of-presentation transitions to complete properly. For me with this MR it's still at SESSION_STATE_STOPPING_SINKS when session_stop() is called. Note that you need !608 to use this program. P.S. please rename requests_callback to sa_ready_callback or something similar, it's about checking for pending samples, and not about making new requests. [MFPlayer.exe](/uploads/bd4e0e1f11666eaf435a6ad54045655e/MFPlayer.exe)
I've been looking into that, and I now think I better understand the logic.
There's two different issues at play, one is the SA_READY_COMMAND, which is currently unconditionally queueing callbacks commands into the task queue, causing race conditions when other commands such as START or END are pending. I think this issue is solved by making the sample allocator ready notifications a separate callback.
Then, there's the handling of the END command. I believe that regardless of how we keep track of the pending command, it'll still be racy the way it's currently done. For instance, consider the following sequence:
Initially the command queue is empty, the session is playing.
1. Thread 1 calls IMFMediaSession_Close. 2. Thread 2 receives MEEndOfPresentation.
3. Thread 1 enters session CS.
* Append a CLOSE command. * Queue the callback task to Thread 3.
4. Thread 1 leaves session CS.
5. Thread 3 calls session_commands_callback_Invoke.
6. Thread 2 enters session CS.
* Prepend a END command.
7. Thread 2 leaves session CS.
8. Thread 3 enters session CS.
* Removes the END command, does nothing.
9. Thread 3 leaves session CS.
Thread 3 is now idle as no more callback task has been queued.
Later, when either MEStreamSinkMarker event or any of the MEStreamSink* state events are received, the session will complete the pending command which is now the CLOSE command, but not actually execute it.
I think the same can happen with commands other than Close, and I don't think having a separate pending command would actually change anything here.