On Mon Jun 23 10:22:32 2025 +0000, Conor McCarthy wrote:
> IIRC it was improved by handling MF_E_SHUTDOWN returned by
> BeginGetEvent(), but could still get stuck waiting for an event from the
> source which can't be received because having the event queue call
> Invoke() after shutdown is not Windows behaviour (but did fix the issue
> in testing).
Robustness against unexpected shutdown in Windows has something to do with the source, because when I tested it with a custom source, the Windows media session would hang.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107612
On Mon Jun 23 10:20:43 2025 +0000, Nikolay Sivov wrote:
> @rbernon we don't need to remove it in this MR, but other comments apply regardless.
IIRC it was improved by handling MF_E_SHUTDOWN returned by BeginGetEvent(), but could still get stuck waiting for an event from the source which can't be received because having the event queue call Invoke() after shutdown is not Windows behaviour (but did fix the issue in testing).
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107611
On Mon Jun 23 10:13:20 2025 +0000, Nikolay Sivov wrote:
> Official code samples for Media Foundation sources do not have anything
> like that. If there is a pending request we could respond with
> MEMediaSample(MF_E_SHUTDOWN), and any following RequestSample will
> return MF_E_SHUTDOWN. Is this not enough?
@rbernon we don't need to remove it in this MR, but other comments apply regardless.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107610
On Mon Jun 23 10:09:37 2025 +0000, Conor McCarthy wrote:
> I wasn't able to prove it uses a callback, but it definitely has some
> means for notifying the session, and it wasn't the event queue.
Official code samples for Media Foundation sources do not have anything like that. If there is a pending request we could respond with MEMediaSample(MF_E_SHUTDOWN), and any following RequestSample will return MF_E_SHUTDOWN. Is this not enough?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107609
On Mon Jun 23 10:02:49 2025 +0000, Rémi Bernon wrote:
> > And we should get rid of IMFMediaShutdownNotify, I think I missed when
> it was added. We should not need any custom interfaces to handle this.
> IIRC it was required because shutting down the source also shuts down
> its event queue, it's then not possible for the session to be reliably
> notified of the shutdown without a dedicated callback. I believe
> @cmccarthy checked that it's also using a direct callback on Windows.
I wasn't able to prove it uses a callback, but it definitely has some means for notifying the session, and it wasn't the event queue.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107608
> And we should get rid of IMFMediaShutdownNotify, I think I missed when it was added. We should not need any custom interfaces to handle this.
IIRC it was required because shutting down the source also shuts down its event queue, it's then not possible for the session to be reliably notified of the shutdown without a dedicated callback. I believe @cmccarthy checked that it's also using a direct callback on Windows.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107607
We currently call session_handle_source_shutdown() for any MF_E_SHUTDOWN return value coming from any node. We should probably limit this to sources.
And we should get rid of IMFMediaShutdownNotify, I think I missed when it was added. We should not need any custom interfaces to handle this.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107605
Nikolay Sivov (@nsivov) commented about dlls/mf/session.c:
> }
>
> session->source_shutdown_handled = FALSE;
> + if (session->state == SESSION_STATE_SOURCE_SHUTDOWN)
> + session->state = SESSION_STATE_STOPPED;
>
Should we merge "source_shutdown_handled" into this new state?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/8402#note_107604