[PATCH v6 0/7] MR140: qasf: More ASF reader filter implementation.
-- v6: qasf: Configure WMReader stream selection in asf_reader_init_stream. qasf: Configure WMReader stream format in asf_reader_init_stream. winegstreamer: Implement IWMOutputMediaProps_SetMediaType. wmvcore/tests: Add some IWMOutputMediaProps_SetMediaType tests. qasf: Implement ASF Reader filter init_stream and cleanup_stream. qasf: Implement ASF Reader filter pin DecideBufferSize. qasf: Implement ASF Reader filter pin_get_media_type. https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> And name pins accordingly. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/qasf/asfreader.c | 58 ++++++++++++++++++++++++++++++++++++- dlls/qasf/tests/asfreader.c | 4 +-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/dlls/qasf/asfreader.c b/dlls/qasf/asfreader.c index 13b6e2d7bbe..8c042e72773 100644 --- a/dlls/qasf/asfreader.c +++ b/dlls/qasf/asfreader.c @@ -48,6 +48,52 @@ struct asf_reader struct asf_stream streams[16]; }; +static inline struct asf_stream *impl_from_strmbase_pin(struct strmbase_pin *iface) +{ + return CONTAINING_RECORD(iface, struct asf_stream, source.pin); +} + +static inline struct asf_reader *asf_reader_from_asf_stream(struct asf_stream *stream) +{ + return CONTAINING_RECORD(stream, struct asf_reader, streams[stream->index]); +} + +static HRESULT asf_stream_get_media_type(struct strmbase_pin *iface, unsigned int index, AM_MEDIA_TYPE *media_type) +{ + struct asf_stream *stream = impl_from_strmbase_pin(iface); + struct asf_reader *filter = asf_reader_from_asf_stream(stream); + IWMOutputMediaProps *props; + AM_MEDIA_TYPE *mt; + DWORD size; + HRESULT hr; + + TRACE("iface %p, index %u, media_type %p.\n", iface, index, media_type); + + if (FAILED(IWMReader_GetOutputFormat(filter->reader, stream->index, index, &props))) + return VFW_S_NO_MORE_ITEMS; + + if (FAILED(hr = IWMOutputMediaProps_GetMediaType(props, NULL, &size))) + { + IWMOutputMediaProps_Release(props); + return hr; + } + + if (!(mt = malloc(size))) + { + IWMOutputMediaProps_Release(props); + return E_OUTOFMEMORY; + } + + hr = IWMOutputMediaProps_GetMediaType(props, (WM_MEDIA_TYPE *)mt, &size); + if (SUCCEEDED(hr)) + hr = CopyMediaType(media_type, mt); + + free(mt); + + IWMOutputMediaProps_Release(props); + return hr; +} + static inline struct asf_reader *impl_from_strmbase_filter(struct strmbase_filter *iface) { return CONTAINING_RECORD(iface, struct asf_reader, filter); @@ -120,6 +166,7 @@ static HRESULT WINAPI asf_reader_DecideBufferSize(struct strmbase_source *iface, static const struct strmbase_source_ops source_ops = { + .base.pin_get_media_type = asf_stream_get_media_type, .pfnDecideAllocator = BaseOutputPinImpl_DecideAllocator, .pfnAttemptConnection = BaseOutputPinImpl_AttemptConnection, .pfnDecideBufferSize = asf_reader_DecideBufferSize, @@ -275,6 +322,7 @@ static HRESULT WINAPI reader_callback_OnStatus(IWMReaderCallback *iface, WMT_STA WMT_ATTR_DATATYPE type, BYTE *value, void *context) { struct asf_reader *filter = impl_from_IWMReaderCallback(iface)->filter; + AM_MEDIA_TYPE stream_media_type = {0}; DWORD i, stream_count; WCHAR name[MAX_PATH]; @@ -304,7 +352,15 @@ static HRESULT WINAPI reader_callback_OnStatus(IWMReaderCallback *iface, WMT_STA for (i = 0; i < stream_count; ++i) { struct asf_stream *stream = filter->streams + i; - swprintf(name, ARRAY_SIZE(name), L"Raw Stream %u", stream->index); + + if (FAILED(hr = asf_stream_get_media_type(&stream->source.pin, 0, &stream_media_type))) + WARN("Failed to get stream media type, hr %#lx.\n", hr); + if (IsEqualGUID(&stream_media_type.majortype, &MEDIATYPE_Video)) + swprintf(name, ARRAY_SIZE(name), L"Raw Video %u", stream->index); + else + swprintf(name, ARRAY_SIZE(name), L"Raw Audio %u", stream->index); + FreeMediaType(&stream_media_type); + strmbase_source_init(&stream->source, &filter->filter, name, &source_ops); } filter->stream_count = stream_count; diff --git a/dlls/qasf/tests/asfreader.c b/dlls/qasf/tests/asfreader.c index 494d0384ac8..ed78a856003 100644 --- a/dlls/qasf/tests/asfreader.c +++ b/dlls/qasf/tests/asfreader.c @@ -231,13 +231,11 @@ static void check_pin(IPin *pin, IBaseFilter *expect_filter, PIN_DIRECTION expec ok(hr == S_OK, "Got hr %#lx.\n", hr); ok(info.pFilter == expect_filter, "Got filter %p.\n", info.pFilter); ok(info.dir == expect_dir, "Got dir %#x.\n", info.dir); - todo_wine ok(!wcscmp(info.achName, expect_name), "Got name %s.\n", debugstr_w(info.achName)); IBaseFilter_Release(info.pFilter); hr = IPin_QueryId(pin, &id); ok(hr == S_OK, "Got hr %#lx.\n", hr); - todo_wine ok(!wcscmp(id, expect_id), "Got id %s.\n", debugstr_w(id)); CoTaskMemFree(id); @@ -253,7 +251,7 @@ static void check_pin(IPin *pin, IBaseFilter *expect_filter, PIN_DIRECTION expec FreeMediaType(mt); CoTaskMemFree(mt); } - todo_wine + todo_wine_if(IsEqualGUID(&expect_mt[0].majortype, &MEDIATYPE_Video)) ok(i == expect_mt_count, "Got %u types.\n", i); ok(hr == S_FALSE, "Got hr %#lx.\n", hr); IEnumMediaTypes_Release(enum_mt); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/qasf/asfreader.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/dlls/qasf/asfreader.c b/dlls/qasf/asfreader.c index 8c042e72773..4fa9136982d 100644 --- a/dlls/qasf/asfreader.c +++ b/dlls/qasf/asfreader.c @@ -160,8 +160,29 @@ static const struct strmbase_filter_ops filter_ops = static HRESULT WINAPI asf_reader_DecideBufferSize(struct strmbase_source *iface, IMemAllocator *allocator, ALLOCATOR_PROPERTIES *req_props) { - FIXME("iface %p, allocator %p, req_props %p stub!\n", iface, allocator, req_props); - return E_NOTIMPL; + struct asf_stream *stream = impl_from_strmbase_pin(&iface->pin); + unsigned int buffer_size = 16384; + ALLOCATOR_PROPERTIES ret_props; + + TRACE("iface %p, allocator %p, req_props %p.\n", iface, allocator, req_props); + + if (IsEqualGUID(&stream->source.pin.mt.formattype, &FORMAT_VideoInfo)) + { + VIDEOINFOHEADER *format = (VIDEOINFOHEADER *)stream->source.pin.mt.pbFormat; + buffer_size = format->bmiHeader.biSizeImage; + } + else if (IsEqualGUID(&stream->source.pin.mt.formattype, &FORMAT_WaveFormatEx) + && (IsEqualGUID(&stream->source.pin.mt.subtype, &MEDIASUBTYPE_PCM) + || IsEqualGUID(&stream->source.pin.mt.subtype, &MEDIASUBTYPE_IEEE_FLOAT))) + { + WAVEFORMATEX *format = (WAVEFORMATEX *)stream->source.pin.mt.pbFormat; + buffer_size = format->nAvgBytesPerSec; + } + + req_props->cBuffers = max(req_props->cBuffers, 1); + req_props->cbBuffer = max(req_props->cbBuffer, buffer_size); + req_props->cbAlign = max(req_props->cbAlign, 1); + return IMemAllocator_SetProperties(allocator, req_props, &ret_props); } static const struct strmbase_source_ops source_ops = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/qasf/asfreader.c | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/dlls/qasf/asfreader.c b/dlls/qasf/asfreader.c index 4fa9136982d..db976f36752 100644 --- a/dlls/qasf/asfreader.c +++ b/dlls/qasf/asfreader.c @@ -150,11 +150,66 @@ static HRESULT asf_reader_query_interface(struct strmbase_filter *iface, REFIID return E_NOINTERFACE; } +static HRESULT asf_reader_init_stream(struct strmbase_filter *iface) +{ + struct asf_reader *filter = impl_from_strmbase_filter(iface); + HRESULT hr; + int i; + + TRACE("iface %p\n", iface); + + for (i = 0; i < filter->stream_count; ++i) + { + struct asf_stream *stream = filter->streams + i; + + if (!stream->source.pin.peer) + continue; + + hr = IMemAllocator_Commit(stream->source.pAllocator); + if (FAILED(hr)) + { + WARN("Failed to commit stream %u allocator, hr %#lx\n", i, hr); + continue; + } + + hr = IPin_NewSegment(stream->source.pin.peer, 0, 0, 1); + if (FAILED(hr)) + { + WARN("Failed to start stream %u new segment, hr %#lx\n", i, hr); + continue; + } + } + + return IWMReader_Start(filter->reader, 0, 0, 1, NULL); +} + +static HRESULT asf_reader_cleanup_stream(struct strmbase_filter *iface) +{ + struct asf_reader *filter = impl_from_strmbase_filter(iface); + int i; + + TRACE("iface %p\n", iface); + + for (i = 0; i < filter->stream_count; ++i) + { + struct asf_stream *stream = filter->streams + i; + + if (!stream->source.pin.peer) + continue; + + IMemAllocator_Decommit(stream->source.pAllocator); + } + + return S_OK; +} + static const struct strmbase_filter_ops filter_ops = { .filter_get_pin = asf_reader_get_pin, .filter_destroy = asf_reader_destroy, .filter_query_interface = asf_reader_query_interface, + .filter_init_stream = asf_reader_init_stream, + .filter_cleanup_stream = asf_reader_cleanup_stream, }; static HRESULT WINAPI asf_reader_DecideBufferSize(struct strmbase_source *iface, @@ -390,6 +445,20 @@ static HRESULT WINAPI reader_callback_OnStatus(IWMReaderCallback *iface, WMT_STA BaseFilterImpl_IncrementPinVersion(&filter->filter); break; + case WMT_END_OF_STREAMING: + EnterCriticalSection(&filter->filter.filter_cs); + for (i = 0; i < filter->stream_count; ++i) + { + struct asf_stream *stream = filter->streams + i; + + if (!stream->source.pin.peer) + continue; + + IPin_EndOfStream(stream->source.pin.peer); + } + LeaveCriticalSection(&filter->filter.filter_cs); + break; + default: WARN("Ignoring status %#x.\n", status); break; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/wmvcore/tests/wmvcore.c | 143 +++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index f639184de90..bbac572acb5 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -48,6 +48,61 @@ static BOOL compare_media_types(const WM_MEDIA_TYPE *a, const WM_MEDIA_TYPE *b) && !memcmp(a->pbFormat, b->pbFormat, a->cbFormat); } +static void init_audio_type(WM_MEDIA_TYPE *mt, const GUID *subtype, UINT bits, UINT channels, UINT rate) +{ + WAVEFORMATEX *format = (WAVEFORMATEX *)(mt + 1); + + format->wFormatTag = subtype->Data1; + format->nChannels = channels; + format->nSamplesPerSec = rate; + format->wBitsPerSample = bits; + format->nBlockAlign = format->nChannels * format->wBitsPerSample; + format->nAvgBytesPerSec = format->nSamplesPerSec * format->nBlockAlign / 8; + format->cbSize = sizeof(*format); + + mt->majortype = MEDIATYPE_Audio; + mt->subtype = *subtype; + mt->bFixedSizeSamples = TRUE; + mt->bTemporalCompression = FALSE; + mt->lSampleSize = format->nAvgBytesPerSec; + mt->formattype = FORMAT_WaveFormatEx; + mt->pUnk = NULL; + mt->cbFormat = sizeof(*format); + mt->pbFormat = (BYTE *)format; +} + +static void init_video_type(WM_MEDIA_TYPE *mt, const GUID *subtype, UINT depth, const RECT *rect) +{ + VIDEOINFOHEADER *video_info = (VIDEOINFOHEADER *)(mt + 1); + + video_info->rcSource = *rect; + video_info->rcTarget = *rect; + video_info->dwBitRate = 0; + video_info->dwBitErrorRate = 0; + video_info->AvgTimePerFrame = 0; + video_info->bmiHeader.biSize = sizeof(video_info->bmiHeader); + video_info->bmiHeader.biWidth = rect->right; + video_info->bmiHeader.biHeight = rect->bottom; + video_info->bmiHeader.biPlanes = 1; + video_info->bmiHeader.biBitCount = depth; + video_info->bmiHeader.biCompression = subtype->Data1; + video_info->bmiHeader.biSizeImage = rect->right * rect->bottom * 4; + video_info->bmiHeader.biXPelsPerMeter = 0; + video_info->bmiHeader.biYPelsPerMeter = 0; + video_info->bmiHeader.biClrUsed = 0; + video_info->bmiHeader.biClrImportant = 0; + + mt->majortype = MEDIATYPE_Video; + mt->subtype = *subtype; + mt->bFixedSizeSamples = TRUE; + mt->bTemporalCompression = FALSE; + mt->lSampleSize = video_info->bmiHeader.biSizeImage; + mt->formattype = FORMAT_VideoInfo; + mt->pUnk = NULL; + mt->cbFormat = sizeof(*video_info); + mt->pbFormat = (BYTE *)video_info; +} + static WCHAR *load_resource(const WCHAR *name) { static WCHAR pathW[MAX_PATH]; @@ -1239,6 +1294,44 @@ static void test_sync_reader_types(void) ok(IsEqualGUID(&majortype2, &majortype), "Expected major type %s, got %s.\n", debugstr_guid(&majortype), debugstr_guid(&majortype2)); + hr = IWMOutputMediaProps_SetMediaType(output_props, NULL); + todo_wine + ok(hr == E_POINTER, "Got hr %#lx.\n", hr); + + memset(mt2_buffer, 0, sizeof(mt2_buffer)); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == E_FAIL, "Got hr %#lx.\n", hr); + + if (IsEqualGUID(&majortype, &MEDIATYPE_Audio)) + { + WAVEFORMATEX *format = (WAVEFORMATEX *)mt->pbFormat; + init_audio_type(mt2, &MEDIASUBTYPE_IEEE_FLOAT, 32, format->nChannels, format->nSamplesPerSec); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); + todo_wine + ok(hr == NS_E_AUDIO_CODEC_NOT_INSTALLED, "Got hr %#lx.\n", hr); + } + else + { + VIDEOINFO *info = (VIDEOINFO *)mt->pbFormat; + init_video_type(mt2, &MEDIASUBTYPE_AYUV, 32, &info->rcTarget); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); + todo_wine + ok(hr == ASF_E_BADMEDIATYPE, "Got hr %#lx.\n", hr); + } + + hr = IWMOutputMediaProps_SetMediaType(output_props, mt); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ref = IWMOutputMediaProps_Release(output_props); ok(!ref, "Got outstanding refcount %ld.\n", ref); @@ -1293,9 +1386,15 @@ static void test_sync_reader_types(void) ok(IsEqualGUID(&mt->majortype, &majortype), "Got major type %s.\n", debugstr_guid(&mt->majortype)); if (IsEqualGUID(&mt->majortype, &MEDIATYPE_Audio)) + { + ok(IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_PCM), "Got subtype %s.\n", debugstr_guid(&mt->subtype)); check_audio_type(mt); + } else + { + ok(!IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_AYUV), "Got subtype %s.\n", debugstr_guid(&mt->subtype)); check_video_type(mt); + } memset(&majortype2, 0xcc, sizeof(majortype2)); hr = IWMOutputMediaProps_GetType(output_props, &majortype2); @@ -2362,6 +2461,44 @@ static void test_async_reader_types(void) ok(IsEqualGUID(&majortype2, &majortype), "Expected major type %s, got %s.\n", debugstr_guid(&majortype), debugstr_guid(&majortype2)); + hr = IWMOutputMediaProps_SetMediaType(output_props, NULL); + todo_wine + ok(hr == E_POINTER, "Got hr %#lx.\n", hr); + + memset(mt2_buffer, 0, sizeof(mt2_buffer)); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == E_FAIL, "Got hr %#lx.\n", hr); + + if (IsEqualGUID(&majortype, &MEDIATYPE_Audio)) + { + WAVEFORMATEX *format = (WAVEFORMATEX *)mt->pbFormat; + init_audio_type(mt2, &MEDIASUBTYPE_IEEE_FLOAT, 32, format->nChannels, format->nSamplesPerSec); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMReader_SetOutputProps(reader, output_number, output_props); + todo_wine + ok(hr == NS_E_AUDIO_CODEC_NOT_INSTALLED, "Got hr %#lx.\n", hr); + } + else + { + VIDEOINFO *info = (VIDEOINFO *)mt->pbFormat; + init_video_type(mt2, &MEDIASUBTYPE_AYUV, 32, &info->rcTarget); + hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMReader_SetOutputProps(reader, output_number, output_props); + todo_wine + ok(hr == ASF_E_BADMEDIATYPE, "Got hr %#lx.\n", hr); + } + + hr = IWMOutputMediaProps_SetMediaType(output_props, mt); + todo_wine + ok(hr == S_OK, "Got hr %#lx.\n", hr); + hr = IWMReader_SetOutputProps(reader, output_number, output_props); + ok(hr == S_OK, "Got hr %#lx.\n", hr); + ref = IWMOutputMediaProps_Release(output_props); ok(!ref, "Got outstanding refcount %ld.\n", ref); @@ -2438,9 +2575,15 @@ static void test_async_reader_types(void) ok(IsEqualGUID(&mt->majortype, &majortype), "Got major type %s.\n", debugstr_guid(&mt->majortype)); if (IsEqualGUID(&mt->majortype, &MEDIATYPE_Audio)) + { + ok(IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_PCM), "Got subtype %s.\n", debugstr_guid(&mt->subtype)); check_audio_type(mt); + } else + { + ok(!IsEqualGUID(&mt->subtype, &MEDIASUBTYPE_AYUV), "Got subtype %s.\n", debugstr_guid(&mt->subtype)); check_video_type(mt); + } memset(&majortype2, 0xcc, sizeof(majortype2)); hr = IWMOutputMediaProps_GetType(output_props, &majortype2); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wm_reader.c | 14 ++++++++++++-- dlls/wmvcore/tests/wmvcore.c | 10 ---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 03adea8a318..9b541fe1d96 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -116,8 +116,18 @@ static HRESULT WINAPI output_props_GetMediaType(IWMOutputMediaProps *iface, WM_M static HRESULT WINAPI output_props_SetMediaType(IWMOutputMediaProps *iface, WM_MEDIA_TYPE *mt) { - FIXME("iface %p, mt %p, stub!\n", iface, mt); - return E_NOTIMPL; + const struct output_props *props = impl_from_IWMOutputMediaProps(iface); + + TRACE("iface %p, mt %p.\n", iface, mt); + + if (!mt) + return E_POINTER; + + if (!IsEqualGUID(&props->mt.majortype, &mt->majortype)) + return E_FAIL; + + FreeMediaType((AM_MEDIA_TYPE *)&props->mt); + return CopyMediaType((AM_MEDIA_TYPE *)&props->mt, (AM_MEDIA_TYPE *)mt); } static HRESULT WINAPI output_props_GetStreamGroupName(IWMOutputMediaProps *iface, WCHAR *name, WORD *len) diff --git a/dlls/wmvcore/tests/wmvcore.c b/dlls/wmvcore/tests/wmvcore.c index bbac572acb5..89f27d3fefe 100644 --- a/dlls/wmvcore/tests/wmvcore.c +++ b/dlls/wmvcore/tests/wmvcore.c @@ -1295,12 +1295,10 @@ static void test_sync_reader_types(void) debugstr_guid(&majortype), debugstr_guid(&majortype2)); hr = IWMOutputMediaProps_SetMediaType(output_props, NULL); - todo_wine ok(hr == E_POINTER, "Got hr %#lx.\n", hr); memset(mt2_buffer, 0, sizeof(mt2_buffer)); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == E_FAIL, "Got hr %#lx.\n", hr); if (IsEqualGUID(&majortype, &MEDIATYPE_Audio)) @@ -1308,7 +1306,6 @@ static void test_sync_reader_types(void) WAVEFORMATEX *format = (WAVEFORMATEX *)mt->pbFormat; init_audio_type(mt2, &MEDIASUBTYPE_IEEE_FLOAT, 32, format->nChannels, format->nSamplesPerSec); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); todo_wine @@ -1319,7 +1316,6 @@ static void test_sync_reader_types(void) VIDEOINFO *info = (VIDEOINFO *)mt->pbFormat; init_video_type(mt2, &MEDIASUBTYPE_AYUV, 32, &info->rcTarget); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); todo_wine @@ -1327,7 +1323,6 @@ static void test_sync_reader_types(void) } hr = IWMOutputMediaProps_SetMediaType(output_props, mt); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMSyncReader_SetOutputProps(reader, output_number, output_props); ok(hr == S_OK, "Got hr %#lx.\n", hr); @@ -2462,12 +2457,10 @@ static void test_async_reader_types(void) debugstr_guid(&majortype), debugstr_guid(&majortype2)); hr = IWMOutputMediaProps_SetMediaType(output_props, NULL); - todo_wine ok(hr == E_POINTER, "Got hr %#lx.\n", hr); memset(mt2_buffer, 0, sizeof(mt2_buffer)); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == E_FAIL, "Got hr %#lx.\n", hr); if (IsEqualGUID(&majortype, &MEDIATYPE_Audio)) @@ -2475,7 +2468,6 @@ static void test_async_reader_types(void) WAVEFORMATEX *format = (WAVEFORMATEX *)mt->pbFormat; init_audio_type(mt2, &MEDIASUBTYPE_IEEE_FLOAT, 32, format->nChannels, format->nSamplesPerSec); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMReader_SetOutputProps(reader, output_number, output_props); todo_wine @@ -2486,7 +2478,6 @@ static void test_async_reader_types(void) VIDEOINFO *info = (VIDEOINFO *)mt->pbFormat; init_video_type(mt2, &MEDIASUBTYPE_AYUV, 32, &info->rcTarget); hr = IWMOutputMediaProps_SetMediaType(output_props, mt2); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMReader_SetOutputProps(reader, output_number, output_props); todo_wine @@ -2494,7 +2485,6 @@ static void test_async_reader_types(void) } hr = IWMOutputMediaProps_SetMediaType(output_props, mt); - todo_wine ok(hr == S_OK, "Got hr %#lx.\n", hr); hr = IWMReader_SetOutputProps(reader, output_number, output_props); ok(hr == S_OK, "Got hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/qasf/asfreader.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/qasf/asfreader.c b/dlls/qasf/asfreader.c index db976f36752..6db1a5ca01f 100644 --- a/dlls/qasf/asfreader.c +++ b/dlls/qasf/asfreader.c @@ -161,6 +161,7 @@ static HRESULT asf_reader_init_stream(struct strmbase_filter *iface) for (i = 0; i < filter->stream_count; ++i) { struct asf_stream *stream = filter->streams + i; + IWMOutputMediaProps *props; if (!stream->source.pin.peer) continue; @@ -178,6 +179,23 @@ static HRESULT asf_reader_init_stream(struct strmbase_filter *iface) WARN("Failed to start stream %u new segment, hr %#lx\n", i, hr); continue; } + + hr = IWMReader_GetOutputFormat(filter->reader, stream->index, 0, &props); + if (FAILED(hr)) + { + WARN("Failed to get stream %u output format, hr %#lx\n", i, hr); + continue; + } + + hr = IWMOutputMediaProps_SetMediaType(props, (WM_MEDIA_TYPE *)&stream->source.pin.mt); + if (SUCCEEDED(hr)) + hr = IWMReader_SetOutputProps(filter->reader, stream->index, props); + IWMOutputMediaProps_Release(props); + if (FAILED(hr)) + { + WARN("Failed to set stream %u output format, hr %#lx\n", i, hr); + continue; + } } return IWMReader_Start(filter->reader, 0, 0, 1, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
From: Rémi Bernon <rbernon(a)codeweavers.com> Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/qasf/asfreader.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/dlls/qasf/asfreader.c b/dlls/qasf/asfreader.c index 6db1a5ca01f..0fcd2d0848c 100644 --- a/dlls/qasf/asfreader.c +++ b/dlls/qasf/asfreader.c @@ -153,16 +153,25 @@ static HRESULT asf_reader_query_interface(struct strmbase_filter *iface, REFIID static HRESULT asf_reader_init_stream(struct strmbase_filter *iface) { struct asf_reader *filter = impl_from_strmbase_filter(iface); + WMT_STREAM_SELECTION selections[ARRAY_SIZE(filter->streams)]; + WORD stream_numbers[ARRAY_SIZE(filter->streams)]; + IWMReaderAdvanced *reader_advanced; HRESULT hr; int i; TRACE("iface %p\n", iface); + if (FAILED(hr = IWMReader_QueryInterface(filter->reader, &IID_IWMReaderAdvanced, (void **)&reader_advanced))) + return hr; + for (i = 0; i < filter->stream_count; ++i) { struct asf_stream *stream = filter->streams + i; IWMOutputMediaProps *props; + stream_numbers[i] = i + 1; + selections[i] = WMT_OFF; + if (!stream->source.pin.peer) continue; @@ -196,8 +205,18 @@ static HRESULT asf_reader_init_stream(struct strmbase_filter *iface) WARN("Failed to set stream %u output format, hr %#lx\n", i, hr); continue; } + + selections[i] = WMT_ON; } + hr = IWMReaderAdvanced_SetStreamsSelected(reader_advanced, filter->stream_count, stream_numbers, selections); + if (FAILED(hr)) + { + WARN("Failed to set stream selection, hr %#lx\n", hr); + } + + IWMReaderAdvanced_Release(reader_advanced); + return IWMReader_Start(filter->reader, 0, 0, 1, NULL); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/140
v6: Add some `IWMOutputMediaProps_SetMediaType` tests, fold loops together. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/140#note_3327
We don't wait for the streaming thread while holding stream_cs, though. Do you mean the reader CS? If we're taking the reader CS from the stream thread, that's also a bug that'll need to be solved somehow.
We wait for the streaming thread, in WMReader_Stop, so while holding the filter_cs. Which may block the streaming thread if the callbacks need to enter the filter_cs as well.
There's a (unfortunately as yet unwritten) rule that nothing from the streaming thread is allowed to ever grab the filter CS.
I failed to notice this during review the first time, but that means that patch 3/7 is broken. It shouldn't be necessary to take the filter CS, though, since pin connection state can't change while the filter is running.
It is completely non obvious, and imho very brittle. I don't think the WMReader_Stop call should ever block and wait for the streaming thread. The reader is asynchronous after all, it should notify the thread and the callbacks would be called eventually. The tests clearly suggest that native doesn't block there, as there's an event and a WaitForSingleObject after the Stop call. Changing the timeout also shows that the call is fully async. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/140#note_3380
On 7/5/22 14:02, Rémi Bernon (@rbernon) wrote:
We don't wait for the streaming thread while holding stream_cs, though. Do you mean the reader CS? If we're taking the reader CS from the stream thread, that's also a bug that'll need to be solved somehow.
We wait for the streaming thread, in WMReader_Stop, so while holding the filter_cs. Which may block the streaming thread if the callbacks need to enter the filter_cs as well.
There's a (unfortunately as yet unwritten) rule that nothing from the streaming thread is allowed to ever grab the filter CS.
I failed to notice this during review the first time, but that means that patch 3/7 is broken. It shouldn't be necessary to take the filter CS, though, since pin connection state can't change while the filter is running.
It is completely non obvious, and imho very brittle. I don't think the WMReader_Stop call should ever block and wait for the streaming thread. The reader is asynchronous after all, it should notify the thread and the callbacks would be called eventually.
The tests clearly suggest that native doesn't block there, as there's an event and a WaitForSingleObject after the Stop call. Changing the timeout also shows that the call is fully async.
It is brittle, and that's why DirectShow is terrible :-) The above message seems to contain some degree of confusion. There are essentially two places where we do the same thing: * in DirectShow, we must clean up the streaming thread in IMediaFilter::Stop(), and wait for it to complete. In strmbase terms this translates to the cleanup_stream() callback. The API requires this. * in wmvcore, we currently wait for the streaming thread in IWMReader::Stop(). As you correctly point out, the API actually *doesn't* require this. However, we are almost certainly going to need to make sure the streaming thread is stopped before destroying the reader object, and some tests imply that it should be done in IWMReader::Close() as well [namely, WMT_STOPPED will not be received after IWMReader::Close().] So yes, we could not block in IWMReader::Stop(), but that's not ultimately going to help anything. We do still need to wait for the streaming thread to stop at some point.
So yes, we could not block in IWMReader::Stop(), but that's not ultimately going to help anything. We do still need to wait for the streaming thread to stop at some point.
Well we could then call `Stop` and wait for the WMT_STOPPED callback notification with a condition variable, releasing the filter_cs so the callbacks are free to enter it to make the thread-safety obviously correct. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/140#note_3436
On 7/6/22 05:08, Rémi Bernon (@rbernon) wrote:
So yes, we could not block in IWMReader::Stop(), but that's not ultimately going to help anything. We do still need to wait for the streaming thread to stop at some point.
Well we could then call `Stop` and wait for the WMT_STOPPED callback notification with a condition variable, releasing the filter_cs so the callbacks are free to enter it to make the thread-safety obviously correct.
Unfortunately we can't release the filter CS while waiting for the thread to stop. We need to protect the whole operation.
On Wed Jul 6 17:52:44 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list: ``` On 7/6/22 05:08, Rémi Bernon (@rbernon) wrote:
So yes, we could not block in IWMReader::Stop(), but that's not ultimately going to help anything. We do still need to wait for the streaming thread to stop at some point.
Well we could then call `Stop` and wait for the WMT_STOPPED callback notification with a condition variable, releasing the filter_cs so the callbacks are free to enter it to make the thread-safety obviously correct. Unfortunately we can't release the filter CS while waiting for the thread to stop. We need to protect the whole operation.
I opened https://gitlab.winehq.org/wine/wine/-/merge_requests/393#note_3473 to make WM async reader state transitions asynchronous, so that we can then do the right thing here and wait for the callback notifications while entering the CS and avoiding the deadlock.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/140#note_3475
participants (2)
-
Rémi Bernon -
Zebediah Figura