[PATCH 0/1] MR4943: mfreadwrite: Replace presentation clock with clock state sink for writer.
There is no need to store a presentation clock object. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943
From: Ziqing Hui <zhui(a)codeweavers.com> There is no need to store a presentation clock object. --- dlls/mfreadwrite/writer.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/dlls/mfreadwrite/writer.c b/dlls/mfreadwrite/writer.c index c708664d2fc..b037c7901de 100644 --- a/dlls/mfreadwrite/writer.c +++ b/dlls/mfreadwrite/writer.c @@ -77,7 +77,6 @@ struct sink_writer DWORD next_id; } streams; - IMFPresentationClock *clock; IMFMediaSink *sink; enum writer_state state; HRESULT status; @@ -221,8 +220,6 @@ static ULONG WINAPI sink_writer_Release(IMFSinkWriter *iface) if (!refcount) { - if (writer->clock) - IMFPresentationClock_Release(writer->clock); if (writer->sink) IMFMediaSink_Release(writer->sink); if (writer->callback) @@ -312,28 +309,10 @@ static HRESULT WINAPI sink_writer_SetInputMediaType(IMFSinkWriter *iface, DWORD return E_NOTIMPL; } -static HRESULT sink_writer_set_presentation_clock(struct sink_writer *writer) -{ - IMFPresentationTimeSource *time_source = NULL; - HRESULT hr; - - if (FAILED(hr = MFCreatePresentationClock(&writer->clock))) return hr; - - if (FAILED(IMFMediaSink_QueryInterface(writer->sink, &IID_IMFPresentationTimeSource, (void **)&time_source))) - hr = MFCreateSystemTimeSource(&time_source); - - if (SUCCEEDED(hr = IMFPresentationClock_SetTimeSource(writer->clock, time_source))) - hr = IMFMediaSink_SetPresentationClock(writer->sink, writer->clock); - - if (time_source) - IMFPresentationTimeSource_Release(time_source); - - return hr; -} - static HRESULT WINAPI sink_writer_BeginWriting(IMFSinkWriter *iface) { struct sink_writer *writer = impl_from_IMFSinkWriter(iface); + IMFClockStateSink *state_sink = NULL; HRESULT hr = S_OK; unsigned int i; @@ -341,11 +320,9 @@ static HRESULT WINAPI sink_writer_BeginWriting(IMFSinkWriter *iface) EnterCriticalSection(&writer->cs); - if (!writer->streams.count) - hr = MF_E_INVALIDREQUEST; - else if (writer->state != SINK_WRITER_STATE_INITIAL) + if (!writer->streams.count || writer->state != SINK_WRITER_STATE_INITIAL) hr = MF_E_INVALIDREQUEST; - else if (SUCCEEDED(hr = sink_writer_set_presentation_clock(writer))) + else if (SUCCEEDED(hr = IMFMediaSink_QueryInterface(writer->sink, &IID_IMFClockStateSink, (void **)&state_sink))) { for (i = 0; i < writer->streams.count; ++i) { @@ -362,11 +339,14 @@ static HRESULT WINAPI sink_writer_BeginWriting(IMFSinkWriter *iface) } if (SUCCEEDED(hr)) - hr = IMFPresentationClock_Start(writer->clock, 0); + hr = IMFClockStateSink_OnClockStart(state_sink, MFGetSystemTime(), 0); writer->state = SINK_WRITER_STATE_WRITING; } + if (state_sink) + IMFClockStateSink_Release(state_sink); + LeaveCriticalSection(&writer->cs); return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4943
How did you verify this? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59112
On Sat Jan 27 02:36:33 2024 +0000, Nikolay Sivov wrote:
How did you verify this? In our writer, the presentation clock is only needed in BeginWriting(). The purpose of it is to start the media sink, making media sink get into started state. And I don't find any other usages of the presentation clock (Did I miss other usages?).
To start media sink, calling OnClockStart() is enough, we don't need presentation clock. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59242
On Sat Jan 27 02:36:33 2024 +0000, Ziqing Hui wrote:
In our writer, the presentation clock is only needed in BeginWriting(). The purpose of it is to start the media sink, making media sink get into started state. And I don't find any other usages of the presentation clock (Did I miss other usages?). To start media sink, calling OnClockStart() is enough, we don't need presentation clock. I think we don't need presentation clock to do operations on media sink. We can do things directly from the interfaces quried from media sink.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59243
On Sat Jan 27 02:38:23 2024 +0000, Ziqing Hui wrote:
I think we don't need presentation clock to do operations on media sink. We can do things directly from the interfaces quried from media sink. MSDN says media sink should always have IMFClockStateSink interface exposed:
https://learn.microsoft.com/en-us/windows/win32/medfound/media-sinks#media-s... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59244
On Sat Jan 27 02:43:55 2024 +0000, Ziqing Hui wrote:
MSDN says media sink should always have IMFClockStateSink interface exposed: https://learn.microsoft.com/en-us/windows/win32/medfound/media-sinks#media-s... Calling IMFPresentationClock_Start() on presentation clock finally get it invoke IMFClockStateSink_OnClockStart() which is what media sink truely need.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59245
On Sat Jan 27 02:47:39 2024 +0000, Ziqing Hui wrote:
Calling IMFPresentationClock_Start() on presentation clock finally get it invoke IMFClockStateSink_OnClockStart() which is what media sink truely need. The only thing that matters is whether the clock is set on Windows. That's why I asked if you verified this change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59323
On Mon Jan 29 09:35:05 2024 +0000, Nikolay Sivov wrote:
The only thing that matters is whether the clock is set on Windows. That's why I asked if you verified this change. OK, I made some tests today. Looks like GetPresentationClock() returns a valid clock. So the clock is set on Windows. I'll close this MR.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59467
This merge request was closed by Ziqing Hui. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943
On Tue Jan 30 08:40:07 2024 +0000, Ziqing Hui wrote:
OK, I made some tests today. Looks like GetPresentationClock() returns a valid clock. So the clock is set on Windows. I'll close this MR. You can also use MFCreateSinkWriterFromMediaSink() with a custom sink stub and see what happens. It's a legitimate use case, because sinks are supposed to be external plugin-like objects.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59468
On Tue Jan 30 08:50:22 2024 +0000, Nikolay Sivov wrote:
You can also use MFCreateSinkWriterFromMediaSink() with a custom sink stub and see what happens. It's a legitimate use case, because sinks are supposed to be external plugin-like objects. I created a writer from mp4 media sink. Before calling BeginWriting() on writer, GetPresentationClock() on that sink returned MF_E_NO_CLOCK. After BeginWriting(), it returns a valid clock.
The test code is attacted: [clock_test.c](/uploads/27be06ce75c1c05e51fe04a6bde08b13/clock_test.c). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59568
On Wed Jan 31 02:23:16 2024 +0000, Ziqing Hui wrote:
I created a writer from mp4 media sink. Before calling BeginWriting() on writer, GetPresentationClock() on that sink returned MF_E_NO_CLOCK. After BeginWriting(), it returns a valid clock. The test code is attacted: [clock_test.c](/uploads/27be06ce75c1c05e51fe04a6bde08b13/clock_test.c). Oh, I got a question not relating to this MR: Where did the sink class factory guids in mfinternal.idl come from? Did we generate it by ourself?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4943#note_59569
participants (3)
-
Nikolay Sivov (＠nsivov) -
Ziqing Hui
-
Ziqing Hui (＠zhui)