[PATCH v2 1/4] winegstreamer: Introduce audio conversion transform.
Serves as a wrapper of audioconvert, and roughly fills the roll of Windows' CLSID_CResamplerMediaObject to convert audio to the format accepted by the streaming audio renderer. Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/audioconvert.c | 378 +++++++++++++++++++ dlls/winegstreamer/gst_private.h | 2 + dlls/winegstreamer/mfplat.c | 77 ++++ dlls/winegstreamer/winegstreamer_classes.idl | 6 + 5 files changed, 464 insertions(+) create mode 100644 dlls/winegstreamer/audioconvert.c diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index e578d194f7f..0b3229160b9 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -6,6 +6,7 @@ EXTRALIBS = $(GSTREAMER_LIBS) $(PTHREAD_LIBS) PARENTSRC = ../strmbase C_SRCS = \ + audioconvert.c \ filter.c \ gst_cbs.c \ gstdemux.c \ diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c new file mode 100644 index 00000000000..91fa556cb88 --- /dev/null +++ b/dlls/winegstreamer/audioconvert.c @@ -0,0 +1,378 @@ +#include "config.h" + +#include "gst_private.h" + +#include "mfapi.h" +#include "mferror.h" +#include "mfidl.h" + +#include "wine/debug.h" +#include "wine/heap.h" + +WINE_DEFAULT_DEBUG_CHANNEL(mfplat); + +static const GUID *raw_types[] = { + &MFAudioFormat_PCM, + &MFAudioFormat_Float, +}; + +struct audio_converter +{ + IMFTransform IMFTransform_iface; + LONG refcount; +}; + +static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) +{ + return CONTAINING_RECORD(iface, struct audio_converter, IMFTransform_iface); +} + +static HRESULT WINAPI audio_converter_QueryInterface(IMFTransform *iface, REFIID riid, void **obj) +{ + TRACE("%p, %s, %p.\n", iface, debugstr_guid(riid), obj); + + if (IsEqualIID(riid, &IID_IMFTransform) || + IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IMFTransform_AddRef(iface); + return S_OK; + } + + WARN("Unsupported %s.\n", debugstr_guid(riid)); + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI audio_converter_AddRef(IMFTransform *iface) +{ + struct audio_converter *transform = impl_audio_converter_from_IMFTransform(iface); + ULONG refcount = InterlockedIncrement(&transform->refcount); + + TRACE("%p, refcount %u.\n", iface, refcount); + + return refcount; +} + +static ULONG WINAPI audio_converter_Release(IMFTransform *iface) +{ + struct audio_converter *transform = impl_audio_converter_from_IMFTransform(iface); + ULONG refcount = InterlockedDecrement(&transform->refcount); + + TRACE("%p, refcount %u.\n", iface, refcount); + + if (!refcount) + { + heap_free(transform); + } + + return refcount; +} + +static HRESULT WINAPI audio_converter_GetStreamLimits(IMFTransform *iface, DWORD *input_minimum, DWORD *input_maximum, + DWORD *output_minimum, DWORD *output_maximum) +{ + TRACE("%p, %p, %p, %p, %p.\n", iface, input_minimum, input_maximum, output_minimum, output_maximum); + + *input_minimum = *input_maximum = *output_minimum = *output_maximum = 1; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetStreamCount(IMFTransform *iface, DWORD *inputs, DWORD *outputs) +{ + TRACE("%p, %p, %p.\n", iface, inputs, outputs); + + *inputs = *outputs = 1; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetStreamIDs(IMFTransform *iface, DWORD input_size, DWORD *inputs, + DWORD output_size, DWORD *outputs) +{ + TRACE("%p %u %p %u %p.\n", iface, input_size, inputs, output_size, outputs); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStreamInfo(IMFTransform *iface, DWORD id, MFT_INPUT_STREAM_INFO *info) +{ + FIXME("%p %u %p.\n", iface, id, info); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStreamInfo(IMFTransform *iface, DWORD id, MFT_OUTPUT_STREAM_INFO *info) +{ + FIXME("%p %u %p.\n", iface, id, info); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetAttributes(IMFTransform *iface, IMFAttributes **attributes) +{ + FIXME("%p, %p.\n", iface, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStreamAttributes(IMFTransform *iface, DWORD id, + IMFAttributes **attributes) +{ + FIXME("%p, %u, %p.\n", iface, id, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStreamAttributes(IMFTransform *iface, DWORD id, + IMFAttributes **attributes) +{ + FIXME("%p, %u, %p.\n", iface, id, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_DeleteInputStream(IMFTransform *iface, DWORD id) +{ + TRACE("%p, %u.\n", iface, id); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_AddInputStreams(IMFTransform *iface, DWORD streams, DWORD *ids) +{ + TRACE("%p, %u, %p.\n", iface, streams, ids); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputAvailableType(IMFTransform *iface, DWORD id, DWORD index, + IMFMediaType **type) +{ + IMFMediaType *ret; + HRESULT hr; + + TRACE("%p, %u, %u, %p.\n", iface, id, index, type); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (index >= ARRAY_SIZE(raw_types)) + return MF_E_NO_MORE_TYPES; + + if (FAILED(hr = MFCreateMediaType(&ret))) + return hr; + + if (FAILED(hr = IMFMediaType_SetGUID(ret, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio))) + { + IMFMediaType_Release(ret); + return hr; + } + + if (FAILED(hr = IMFMediaType_SetGUID(ret, &MF_MT_SUBTYPE, raw_types[index]))) + { + IMFMediaType_Release(ret); + return hr; + } + + *type = ret; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface, DWORD id, DWORD index, + IMFMediaType **type) +{ + IMFMediaType *output_type; + HRESULT hr; + + static const DWORD rates[] = {44100, 48000}; + static const DWORD channel_cnts[] = {1, 2, 6}; + static const DWORD sizes[] = {16, 24, 32}; + const GUID *subtype; + DWORD rate, channels, bps; + + TRACE("%p, %u, %u, %p.\n", iface, id, index, type); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (index >= (2/*rates*/ * 3/*layouts*/ * 3/*bps PCM*/) + (2 * 3)) + return MF_E_NO_MORE_TYPES; + + if (FAILED(hr = MFCreateMediaType(&output_type))) + return hr; + + if (index < 2 * 3 * 3) + { + subtype = &MFAudioFormat_PCM; + rate = rates[index % 2]; + channels = channel_cnts[(index / 2) % 3]; + bps = sizes[(index / (2*3)) % 3]; + } + else + { + index -= (2 * 3 * 3); + subtype = &MFAudioFormat_Float; + bps = 32; + rate = rates[index % 2]; + channels = channel_cnts[(index / 2) % 3]; + } + + + if (FAILED(hr = IMFMediaType_SetGUID(output_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio))) + goto fail; + if (FAILED(hr = IMFMediaType_SetGUID(output_type, &MF_MT_SUBTYPE, subtype))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, rate))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_NUM_CHANNELS, channels))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, bps))) + goto fail; + + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_BLOCK_ALIGNMENT, channels * bps / 8))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_AVG_BYTES_PER_SECOND, rate * channels * bps / 8))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_CHANNEL_MASK, + channels == 1 ? SPEAKER_FRONT_CENTER : + channels == 2 ? SPEAKER_FRONT_LEFT | SPEAKER_FRONT_RIGHT : + /*channels == 6*/ 0x3F))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_ALL_SAMPLES_INDEPENDENT, TRUE))) + goto fail; + + *type = output_type; + + return S_OK; + fail: + IMFMediaType_Release(output_type); + return hr; +} + +static HRESULT WINAPI audio_converter_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_SetOutputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) +{ + FIXME("%p, %u, %p.\n", iface, id, type); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) +{ + FIXME("%p, %u, %p.\n", iface, id, type); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStatus(IMFTransform *iface, DWORD id, DWORD *flags) +{ + FIXME("%p, %u, %p.\n", iface, id, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStatus(IMFTransform *iface, DWORD *flags) +{ + FIXME("%p, %p.\n", iface, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_SetOutputBounds(IMFTransform *iface, LONGLONG lower, LONGLONG upper) +{ + FIXME("%p, %s, %s.\n", iface, wine_dbgstr_longlong(lower), wine_dbgstr_longlong(upper)); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessEvent(IMFTransform *iface, DWORD id, IMFMediaEvent *event) +{ + TRACE("%p, %u, %p.\n", iface, id, event); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} + +static HRESULT WINAPI audio_converter_ProcessInput(IMFTransform *iface, DWORD id, IMFSample *sample, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, sample, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, + MFT_OUTPUT_DATA_BUFFER *samples, DWORD *status) +{ + FIXME("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + + return E_NOTIMPL; +} + +static const IMFTransformVtbl audio_converter_vtbl = +{ + audio_converter_QueryInterface, + audio_converter_AddRef, + audio_converter_Release, + audio_converter_GetStreamLimits, + audio_converter_GetStreamCount, + audio_converter_GetStreamIDs, + audio_converter_GetInputStreamInfo, + audio_converter_GetOutputStreamInfo, + audio_converter_GetAttributes, + audio_converter_GetInputStreamAttributes, + audio_converter_GetOutputStreamAttributes, + audio_converter_DeleteInputStream, + audio_converter_AddInputStreams, + audio_converter_GetInputAvailableType, + audio_converter_GetOutputAvailableType, + audio_converter_SetInputType, + audio_converter_SetOutputType, + audio_converter_GetInputCurrentType, + audio_converter_GetOutputCurrentType, + audio_converter_GetInputStatus, + audio_converter_GetOutputStatus, + audio_converter_SetOutputBounds, + audio_converter_ProcessEvent, + audio_converter_ProcessMessage, + audio_converter_ProcessInput, + audio_converter_ProcessOutput, +}; + +HRESULT audio_converter_create(REFIID riid, void **ret) +{ + struct audio_converter *object; + HRESULT hr; + + TRACE("%s %p\n", debugstr_guid(riid), ret); + + if (!(object = heap_alloc_zero(sizeof(*object)))) + return E_OUTOFMEMORY; + + object->IMFTransform_iface.lpVtbl = &audio_converter_vtbl; + object->refcount = 1; + + *ret = &object->IMFTransform_iface; + return S_OK; +} diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 28e424439d8..7889c996204 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -84,4 +84,6 @@ IMFSample *mf_sample_from_gst_buffer(GstBuffer *in) DECLSPEC_HIDDEN; HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN; +HRESULT audio_converter_create(REFIID riid, void **ret) DECLSPEC_HIDDEN; + #endif /* __GST_PRIVATE_INCLUDED__ */ diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 3d224a5accc..909fb8b3572 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -405,6 +405,8 @@ failed: static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618, 0x5e5a, 0x468a, {0x9f, 0x15, 0xd8, 0x27, 0xa9, 0xa0, 0x81, 0x62}}; +static GUID CLSID_WINEAudioConverter = {0x6a170414,0xaad9,0x4693,{0xb8,0x06,0x3a,0x0c,0x47,0xc5,0x70,0xd6}}; + static const struct class_object { const GUID *clsid; @@ -414,6 +416,7 @@ class_objects[] = { { &CLSID_VideoProcessorMFT, &video_processor_create }, { &CLSID_GStreamerByteStreamHandler, &winegstreamer_stream_handler_create }, + { &CLSID_WINEAudioConverter, &audio_converter_create }, }; HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj) @@ -442,6 +445,80 @@ HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj) return CLASS_E_CLASSNOTAVAILABLE; } +static WCHAR audio_converterW[] = {'A','u','d','i','o',' ','C','o','n','v','e','r','t','e','r',0}; +const GUID *audio_converter_supported_types[] = +{ + &MFAudioFormat_PCM, + &MFAudioFormat_Float, +}; + +static const struct mft +{ + const GUID *clsid; + const GUID *category; + LPWSTR name; + const UINT32 flags; + const GUID *major_type; + const UINT32 input_types_count; + const GUID **input_types; + const UINT32 output_types_count; + const GUID **output_types; + IMFAttributes *attributes; +} +mfts[] = +{ + { + &CLSID_WINEAudioConverter, + &MFT_CATEGORY_AUDIO_EFFECT, + audio_converterW, + MFT_ENUM_FLAG_SYNCMFT, + &MFMediaType_Audio, + ARRAY_SIZE(audio_converter_supported_types), + audio_converter_supported_types, + ARRAY_SIZE(audio_converter_supported_types), + audio_converter_supported_types, + NULL + }, +}; + +HRESULT mfplat_DllRegisterServer(void) +{ + unsigned int i; + HRESULT hr; + + for (i = 0; i < ARRAY_SIZE(mfts); i++) + { + const struct mft *cur = &mfts[i]; + + MFT_REGISTER_TYPE_INFO *input_types, *output_types; + input_types = heap_alloc(cur->input_types_count * sizeof(input_types[0])); + output_types = heap_alloc(cur->output_types_count * sizeof(output_types[0])); + for (i = 0; i < cur->input_types_count; i++) + { + input_types[i].guidMajorType = *(cur->major_type); + input_types[i].guidSubtype = *(cur->input_types[i]); + } + for (i = 0; i < cur->output_types_count; i++) + { + output_types[i].guidMajorType = *(cur->major_type); + output_types[i].guidSubtype = *(cur->output_types[i]); + } + + hr = MFTRegister(*(cur->clsid), *(cur->category), cur->name, cur->flags, cur->input_types_count, + input_types, cur->output_types_count, output_types, cur->attributes); + + heap_free(input_types); + heap_free(output_types); + + if (FAILED(hr)) + { + FIXME("Failed to register MFT, hr %#x\n", hr); + return hr; + } + } + return S_OK; +} + static const struct { const GUID *subtype; diff --git a/dlls/winegstreamer/winegstreamer_classes.idl b/dlls/winegstreamer/winegstreamer_classes.idl index 1dc4ba9a10b..cf1fc69f38a 100644 --- a/dlls/winegstreamer/winegstreamer_classes.idl +++ b/dlls/winegstreamer/winegstreamer_classes.idl @@ -61,3 +61,9 @@ coclass VideoProcessorMFT {} uuid(317df618-5e5a-468a-9f15-d827a9a08162) ] coclass GStreamerByteStreamHandler {} + +[ + threading(both), + uuid(6a170414-aad9-4693-b806-3a0c47c570d6) +] +coclass WINEAudioConverter { } -- 2.29.2
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/audioconvert.c | 190 +++++++++++++++++++++++++++++- 1 file changed, 185 insertions(+), 5 deletions(-) diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c index 91fa556cb88..04227d168ae 100644 --- a/dlls/winegstreamer/audioconvert.c +++ b/dlls/winegstreamer/audioconvert.c @@ -1,4 +1,5 @@ #include "config.h" +#include <gst/gst.h> #include "gst_private.h" @@ -20,6 +21,10 @@ struct audio_converter { IMFTransform IMFTransform_iface; LONG refcount; + IMFMediaType *input_type; + IMFMediaType *output_type; + CRITICAL_SECTION cs; + BOOL valid_state; }; static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) @@ -63,6 +68,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface) if (!refcount) { + DeleteCriticalSection(&transform->cs); heap_free(transform); } @@ -252,18 +258,191 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface return hr; } +static void audio_converter_update_pipeline_state(struct audio_converter *converter) +{ + GstCaps *input_caps, *output_caps; + + if (!(converter->valid_state = converter->input_type && converter->output_type)) + return; + + if (!(input_caps = caps_from_mf_media_type(converter->input_type))) + { + converter->valid_state = FALSE; + return; + } + if (!(output_caps = caps_from_mf_media_type(converter->output_type))) + { + converter->valid_state = FALSE; + gst_caps_unref(input_caps); + return; + } + + if (TRACE_ON(mfplat)) + { + gchar *input_caps_str, *output_caps_str; + + input_caps_str = gst_caps_to_string(input_caps); + output_caps_str = gst_caps_to_string(output_caps); + + TRACE("Audio converter MFT configured to transform caps %s to caps %s\n", + debugstr_a(input_caps_str), debugstr_a(output_caps_str)); + + g_free(input_caps_str); + g_free(output_caps_str); + } + + gst_caps_unref(input_caps); + gst_caps_unref(output_caps); + + return; +} + static HRESULT WINAPI audio_converter_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + unsigned int i; + HRESULT hr; - return E_NOTIMPL; + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + + TRACE("%p, %u, %p, %#x.\n", iface, id, type, flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (type) + { + GUID major_type, subtype; + BOOL found = FALSE; + DWORD unused; + + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_MAJOR_TYPE, &major_type))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_SUBTYPE, &subtype))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &unused))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &unused))) + return MF_E_INVALIDTYPE; + if (IsEqualGUID(&subtype, &MFAudioFormat_PCM) && FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BITS_PER_SAMPLE, &unused))) + return MF_E_INVALIDTYPE; + + if (!(IsEqualGUID(&major_type, &MFMediaType_Audio))) + return MF_E_INVALIDTYPE; + + for (i = 0; i < ARRAY_SIZE(raw_types); i++) + { + if (IsEqualGUID(&subtype, raw_types[i])) + { + found = TRUE; + break; + } + } + + if (!found) + return MF_E_INVALIDTYPE; + } + + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; + + hr = S_OK; + + EnterCriticalSection(&converter->cs); + + if (type) + { + if (!converter->input_type) + if (FAILED(hr = MFCreateMediaType(&converter->input_type))) + goto done; + + if (FAILED(hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *) converter->input_type))) + goto done; + } + else if (converter->input_type) + { + IMFMediaType_Release(converter->input_type); + converter->input_type = NULL; + } + + done: + if (hr == S_OK) + audio_converter_update_pipeline_state(converter); + LeaveCriticalSection(&converter->cs); + + return S_OK; } static HRESULT WINAPI audio_converter_SetOutputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + GUID major_type, subtype; + unsigned int i; + DWORD unused; + HRESULT hr; - return E_NOTIMPL; + TRACE("%p, %u, %p, %#x.\n", iface, id, type, flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (!converter->input_type) + return MF_E_TRANSFORM_TYPE_NOT_SET; + + if (type) + { + /* validate the type */ + + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_MAJOR_TYPE, &major_type))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_SUBTYPE, &subtype))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &unused))) + return MF_E_INVALIDTYPE; + if (IsEqualGUID(&subtype, &MFAudioFormat_PCM) && FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BITS_PER_SAMPLE, &unused))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &unused))) + return MF_E_INVALIDTYPE; + + if (!(IsEqualGUID(&major_type, &MFMediaType_Audio))) + return MF_E_INVALIDTYPE; + + for (i = 0; i < ARRAY_SIZE(raw_types); i++) + { + if (IsEqualGUID(&subtype, raw_types[i])) + break; + if (i == ARRAY_SIZE(raw_types)) + return MF_E_INVALIDTYPE; + } + } + + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; + + EnterCriticalSection(&converter->cs); + + hr = S_OK; + + if (type) + { + if (!converter->output_type) + if (FAILED(hr = MFCreateMediaType(&converter->output_type))) + goto done; + + if (FAILED(hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *) converter->output_type))) + goto done; + } + else if (converter->output_type) + { + IMFMediaType_Release(converter->output_type); + converter->output_type = NULL; + } + + done: + if (hr == S_OK) + audio_converter_update_pipeline_state(converter); + LeaveCriticalSection(&converter->cs); + + return hr; } static HRESULT WINAPI audio_converter_GetInputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) @@ -363,7 +542,6 @@ static const IMFTransformVtbl audio_converter_vtbl = HRESULT audio_converter_create(REFIID riid, void **ret) { struct audio_converter *object; - HRESULT hr; TRACE("%s %p\n", debugstr_guid(riid), ret); @@ -373,6 +551,8 @@ HRESULT audio_converter_create(REFIID riid, void **ret) object->IMFTransform_iface.lpVtbl = &audio_converter_vtbl; object->refcount = 1; + InitializeCriticalSection(&object->cs); + *ret = &object->IMFTransform_iface; return S_OK; } -- 2.29.2
On 11/18/20 2:52 PM, Derek Lesho wrote:
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/audioconvert.c | 190 +++++++++++++++++++++++++++++- 1 file changed, 185 insertions(+), 5 deletions(-)
As similar as the implementations of these two functions are, splitting the patch probably wouldn't be a bad thing.
diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c index 91fa556cb88..04227d168ae 100644 --- a/dlls/winegstreamer/audioconvert.c +++ b/dlls/winegstreamer/audioconvert.c @@ -1,4 +1,5 @@ #include "config.h" +#include <gst/gst.h>
#include "gst_private.h"
@@ -20,6 +21,10 @@ struct audio_converter { IMFTransform IMFTransform_iface; LONG refcount; + IMFMediaType *input_type; + IMFMediaType *output_type; + CRITICAL_SECTION cs; + BOOL valid_state;
This field is unused in this patch, but is there a reason its uses can't be replaced with "converter->input_type && converter->output_type"?
};
static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) @@ -63,6 +68,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface)
if (!refcount) { + DeleteCriticalSection(&transform->cs); heap_free(transform); }
@@ -252,18 +258,191 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface return hr; }
+static void audio_converter_update_pipeline_state(struct audio_converter *converter) +{ + GstCaps *input_caps, *output_caps; + + if (!(converter->valid_state = converter->input_type && converter->output_type)) + return; + + if (!(input_caps = caps_from_mf_media_type(converter->input_type))) + { + converter->valid_state = FALSE; + return; + } + if (!(output_caps = caps_from_mf_media_type(converter->output_type))) + { + converter->valid_state = FALSE; + gst_caps_unref(input_caps); + return; + } + + if (TRACE_ON(mfplat)) + { + gchar *input_caps_str, *output_caps_str; + + input_caps_str = gst_caps_to_string(input_caps); + output_caps_str = gst_caps_to_string(output_caps); + + TRACE("Audio converter MFT configured to transform caps %s to caps %s\n", + debugstr_a(input_caps_str), debugstr_a(output_caps_str)); + + g_free(input_caps_str); + g_free(output_caps_str); + } + + gst_caps_unref(input_caps); + gst_caps_unref(output_caps); + + return; +} +
This function doesn't really do anything but trace; I'm not sure it makes sense in this patch.
static HRESULT WINAPI audio_converter_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + unsigned int i; + HRESULT hr;
- return E_NOTIMPL; + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + + TRACE("%p, %u, %p, %#x.\n", iface, id, type, flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (type) + { + GUID major_type, subtype; + BOOL found = FALSE; + DWORD unused; + + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_MAJOR_TYPE, &major_type))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_SUBTYPE, &subtype))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &unused))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &unused))) + return MF_E_INVALIDTYPE; + if (IsEqualGUID(&subtype, &MFAudioFormat_PCM) && FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BITS_PER_SAMPLE, &unused))) + return MF_E_INVALIDTYPE; + + if (!(IsEqualGUID(&major_type, &MFMediaType_Audio))) + return MF_E_INVALIDTYPE; + + for (i = 0; i < ARRAY_SIZE(raw_types); i++) + { + if (IsEqualGUID(&subtype, raw_types[i])) + { + found = TRUE; + break; + } + }
I guess I didn't notice this in patch 1/4, but raw_types seems a bit overkill when only two types are used (and it's not clear to me that more will be needed).
+ + if (!found) + return MF_E_INVALIDTYPE; + } + + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK;
Wait, so what happens if MFT_SET_TYPE_TEST_ONLY is used with a NULL type?
+ + hr = S_OK; + + EnterCriticalSection(&converter->cs); + + if (type) + { + if (!converter->input_type) + if (FAILED(hr = MFCreateMediaType(&converter->input_type))) + goto done; + + if (FAILED(hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *) converter->input_type))) + goto done;
This "goto" is redundant. The former could potentially be made redundant as well.
+ } + else if (converter->input_type) + { + IMFMediaType_Release(converter->input_type); + converter->input_type = NULL; + } + + done: + if (hr == S_OK) + audio_converter_update_pipeline_state(converter); + LeaveCriticalSection(&converter->cs); + + return S_OK; }
static HRESULT WINAPI audio_converter_SetOutputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + GUID major_type, subtype; + unsigned int i; + DWORD unused; + HRESULT hr;
- return E_NOTIMPL; + TRACE("%p, %u, %p, %#x.\n", iface, id, type, flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (!converter->input_type) + return MF_E_TRANSFORM_TYPE_NOT_SET; + + if (type) + { + /* validate the type */ + + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_MAJOR_TYPE, &major_type))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetGUID(type, &MF_MT_SUBTYPE, &subtype))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_NUM_CHANNELS, &unused))) + return MF_E_INVALIDTYPE; + if (IsEqualGUID(&subtype, &MFAudioFormat_PCM) && FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_BITS_PER_SAMPLE, &unused))) + return MF_E_INVALIDTYPE; + if (FAILED(IMFMediaType_GetUINT32(type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, &unused))) + return MF_E_INVALIDTYPE; + + if (!(IsEqualGUID(&major_type, &MFMediaType_Audio))) + return MF_E_INVALIDTYPE; + + for (i = 0; i < ARRAY_SIZE(raw_types); i++) + { + if (IsEqualGUID(&subtype, raw_types[i])) + break; + if (i == ARRAY_SIZE(raw_types)) + return MF_E_INVALIDTYPE; + } + } + + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; + + EnterCriticalSection(&converter->cs); + + hr = S_OK; + + if (type) + { + if (!converter->output_type) + if (FAILED(hr = MFCreateMediaType(&converter->output_type))) + goto done; + + if (FAILED(hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *) converter->output_type))) + goto done; + } + else if (converter->output_type) + { + IMFMediaType_Release(converter->output_type); + converter->output_type = NULL; + } + + done: + if (hr == S_OK) + audio_converter_update_pipeline_state(converter); + LeaveCriticalSection(&converter->cs); + + return hr; }
Most of the above comments apply here as well.
static HRESULT WINAPI audio_converter_GetInputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) @@ -363,7 +542,6 @@ static const IMFTransformVtbl audio_converter_vtbl = HRESULT audio_converter_create(REFIID riid, void **ret) { struct audio_converter *object; - HRESULT hr;
This should be part of the previous patch.
TRACE("%s %p\n", debugstr_guid(riid), ret);
@@ -373,6 +551,8 @@ HRESULT audio_converter_create(REFIID riid, void **ret) object->IMFTransform_iface.lpVtbl = &audio_converter_vtbl; object->refcount = 1;
+ InitializeCriticalSection(&object->cs); +
Can you please add debug info to the CS?
*ret = &object->IMFTransform_iface; return S_OK; }
On 12/1/20 3:10 PM, Zebediah Figura (she/her) wrote:
This field is unused in this patch, but is there a reason its uses can't be replaced with "converter->input_type && converter->output_type"? If the functionality of update_pipeline_state were moved to the validation stage of Set(Input/Output)Type, yes, that'd be possible. I'll do that.
};
static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) @@ -63,6 +68,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface)
if (!refcount) { + DeleteCriticalSection(&transform->cs); heap_free(transform); }
@@ -252,18 +258,191 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface return hr; }
+static void audio_converter_update_pipeline_state(struct audio_converter *converter) +{ + GstCaps *input_caps, *output_caps; + + if (!(converter->valid_state = converter->input_type && converter->output_type)) + return; + + if (!(input_caps = caps_from_mf_media_type(converter->input_type))) + { + converter->valid_state = FALSE; + return; + } + if (!(output_caps = caps_from_mf_media_type(converter->output_type))) + { + converter->valid_state = FALSE; + gst_caps_unref(input_caps); + return; + } + + if (TRACE_ON(mfplat)) + { + gchar *input_caps_str, *output_caps_str; + + input_caps_str = gst_caps_to_string(input_caps); + output_caps_str = gst_caps_to_string(output_caps); + + TRACE("Audio converter MFT configured to transform caps %s to caps %s\n", + debugstr_a(input_caps_str), debugstr_a(output_caps_str)); + + g_free(input_caps_str); + g_free(output_caps_str); + } + + gst_caps_unref(input_caps); + gst_caps_unref(output_caps); + + return; +} + This function doesn't really do anything but trace; I'm not sure it makes sense in this patch. I agree, even in the later patches where it does more, it just does the same thing twice for both the input and output type. It makes more sense to include it inline.+ + if (!found) + return MF_E_INVALIDTYPE; + } + + if (flags & MFT_SET_TYPE_TEST_ONLY) + return S_OK; Wait, so what happens if MFT_SET_TYPE_TEST_ONLY is used with a NULL type? S_OK is returned, indicating that you can unset the input type.
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/audioconvert.c | 202 +++++++++++++++++++++++++++++- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/mfplat.c | 75 +++++++++++ 3 files changed, 273 insertions(+), 5 deletions(-) diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c index 04227d168ae..deb5ceb2f7c 100644 --- a/dlls/winegstreamer/audioconvert.c +++ b/dlls/winegstreamer/audioconvert.c @@ -24,7 +24,8 @@ struct audio_converter IMFMediaType *input_type; IMFMediaType *output_type; CRITICAL_SECTION cs; - BOOL valid_state; + BOOL valid_state, inflight; + GstElement *container, *appsrc, *audioconvert, *resampler, *appsink; }; static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) @@ -69,6 +70,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface) if (!refcount) { DeleteCriticalSection(&transform->cs); + gst_object_unref(transform->container); heap_free(transform); } @@ -261,22 +263,54 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface static void audio_converter_update_pipeline_state(struct audio_converter *converter) { GstCaps *input_caps, *output_caps; + GstStructure *input_structure, *output_structure; + guint64 channels_in, channels_out, channel_mask = 0; if (!(converter->valid_state = converter->input_type && converter->output_type)) + { + gst_element_set_state(converter->container, GST_STATE_READY); return; + } if (!(input_caps = caps_from_mf_media_type(converter->input_type))) { converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); return; } if (!(output_caps = caps_from_mf_media_type(converter->output_type))) { converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); + gst_caps_unref(input_caps); + return; + } + + /* audioconvert needs a valid channel-mask */ + input_structure = gst_caps_get_structure(input_caps, 0); + output_structure = gst_caps_get_structure(output_caps, 0); + + if (!gst_structure_get(input_structure, "channels", G_TYPE_INT, &channels_in, NULL) || + !gst_structure_get(output_structure, "channels", G_TYPE_INT, &channels_out, NULL)) + { + converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); gst_caps_unref(input_caps); + gst_caps_unref(output_caps); return; } + gst_structure_get(input_structure, "channel-mask", GST_TYPE_BITMASK, &channel_mask, NULL); + if (channel_mask == 0) + gst_caps_set_simple(input_caps, "channel-mask", GST_TYPE_BITMASK, (guint64) (1 << channels_in) - 1, NULL); + + gst_structure_get(output_structure, "channel-mask", GST_TYPE_BITMASK, &channel_mask, NULL); + if (channel_mask == 0) + gst_caps_set_simple(output_caps, "channel-mask", GST_TYPE_BITMASK, (guint64) (1 << channels_out) - 1, NULL); + + g_object_set(converter->appsrc, "caps", input_caps, NULL); + g_object_set(converter->appsink, "caps", output_caps, NULL); + if (TRACE_ON(mfplat)) { gchar *input_caps_str, *output_caps_str; @@ -293,6 +327,7 @@ static void audio_converter_update_pipeline_state(struct audio_converter *conver gst_caps_unref(input_caps); gst_caps_unref(output_caps); + gst_element_set_state(converter->container, GST_STATE_PLAYING); return; } @@ -496,17 +531,114 @@ static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_ME static HRESULT WINAPI audio_converter_ProcessInput(IMFTransform *iface, DWORD id, IMFSample *sample, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, sample, flags); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + GstBuffer *gst_buffer; + HRESULT hr = S_OK; + int ret; - return E_NOTIMPL; + TRACE("%p, %u, %p, %#x.\n", iface, id, sample, flags); + + if (flags) + WARN("Unsupported flags %#x\n", flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + EnterCriticalSection(&converter->cs); + + if (!converter->valid_state) + { + hr = MF_E_TRANSFORM_TYPE_NOT_SET; + goto done; + } + + if (converter->inflight) + { + hr = MF_E_NOTACCEPTING; + goto done; + } + + if (!(gst_buffer = gst_buffer_from_mf_sample(sample))) + { + hr = E_FAIL; + goto done; + } + + g_signal_emit_by_name(converter->appsrc, "push-buffer", gst_buffer, &ret); + gst_buffer_unref(gst_buffer); + if (ret != GST_FLOW_OK) + { + ERR("Couldn't push buffer ret = %d\n", ret); + hr = E_FAIL; + goto done; + } + + converter->inflight = TRUE; + + done: + LeaveCriticalSection(&converter->cs); + + return hr; } static HRESULT WINAPI audio_converter_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, MFT_OUTPUT_DATA_BUFFER *samples, DWORD *status) { - FIXME("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + MFT_OUTPUT_DATA_BUFFER *relevant_buffer = NULL; + GstSample *sample; + HRESULT hr = S_OK; + unsigned int i; - return E_NOTIMPL; + TRACE("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + + if (flags) + WARN("Unsupported flags %#x\n", flags); + + for (i = 0; i < count; i++) + { + MFT_OUTPUT_DATA_BUFFER *out_buffer = &samples[i]; + + if (out_buffer->dwStreamID != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (relevant_buffer) + return MF_E_INVALIDSTREAMNUMBER; + + relevant_buffer = out_buffer; + } + + if (!relevant_buffer) + return S_OK; + + EnterCriticalSection(&converter->cs); + + if (!converter->valid_state) + { + hr = MF_E_TRANSFORM_TYPE_NOT_SET; + goto done; + } + + if (!converter->inflight) + { + hr = MF_E_TRANSFORM_NEED_MORE_INPUT; + goto done; + } + + g_signal_emit_by_name(converter->appsink, "pull-sample", &sample); + + converter->inflight = FALSE; + + relevant_buffer->pSample = mf_sample_from_gst_buffer(gst_sample_get_buffer(sample)); + gst_sample_unref(sample); + relevant_buffer->dwStatus = S_OK; + relevant_buffer->pEvents = NULL; + *status = 0; + + done: + LeaveCriticalSection(&converter->cs); + + return hr; } static const IMFTransformVtbl audio_converter_vtbl = @@ -542,6 +674,7 @@ static const IMFTransformVtbl audio_converter_vtbl = HRESULT audio_converter_create(REFIID riid, void **ret) { struct audio_converter *object; + HRESULT hr; TRACE("%s %p\n", debugstr_guid(riid), ret); @@ -553,6 +686,65 @@ HRESULT audio_converter_create(REFIID riid, void **ret) InitializeCriticalSection(&object->cs); + object->container = gst_bin_new(NULL); + + if (!(object->appsrc = gst_element_factory_make("appsrc", NULL))) + { + ERR("Failed to create appsrc"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->appsrc); + + if (!(object->audioconvert = gst_element_factory_make("audioconvert", NULL))) + { + ERR("Failed to create converter\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->audioconvert); + + if (!(object->resampler = gst_element_factory_make("audioresample", NULL))) + { + ERR("Failed to create resampler\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->resampler); + + if (!(object->appsink = gst_element_factory_make("appsink", NULL))) + { + ERR("Failed to create appsink\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->appsink); + + if (!(gst_element_link(object->appsrc, object->audioconvert))) + { + ERR("Failed to link appsrc to audioconvert\n"); + hr = E_FAIL; + goto failed; + } + + if (!(gst_element_link(object->audioconvert, object->resampler))) + { + ERR("Failed to link audioconvert to resampler\n"); + hr = E_FAIL; + goto failed; + } + + if (!(gst_element_link(object->resampler, object->appsink))) + { + ERR("Failed to link resampler to appsink\n"); + hr = E_FAIL; + goto failed; + } + *ret = &object->IMFTransform_iface; return S_OK; +failed: + + IMFTransform_Release(&object->IMFTransform_iface); + return hr; } diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 7889c996204..7f96c06dfaf 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -81,6 +81,7 @@ HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HI IMFMediaType *mf_media_type_from_caps(const GstCaps *caps) DECLSPEC_HIDDEN; GstCaps *caps_from_mf_media_type(IMFMediaType *type) DECLSPEC_HIDDEN; IMFSample *mf_sample_from_gst_buffer(GstBuffer *in) DECLSPEC_HIDDEN; +GstBuffer *gst_buffer_from_mf_sample(IMFSample *in) DECLSPEC_HIDDEN; HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 909fb8b3572..7fdf722ec2e 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -877,3 +877,78 @@ done: return out; } + +GstBuffer* gst_buffer_from_mf_sample(IMFSample *mf_sample) +{ + GstBuffer *out = gst_buffer_new(); + IMFMediaBuffer *mf_buffer = NULL; + LONGLONG duration, time; + DWORD buffer_count; + unsigned int i; + HRESULT hr; + + if (FAILED(hr = IMFSample_GetSampleDuration(mf_sample, &duration))) + goto fail; + + if (FAILED(hr = IMFSample_GetSampleTime(mf_sample, &time))) + goto fail; + + GST_BUFFER_DURATION(out) = duration; + GST_BUFFER_PTS(out) = time * 100; + + if (FAILED(hr = IMFSample_GetBufferCount(mf_sample, &buffer_count))) + goto fail; + + for (i = 0; i < buffer_count; i++) + { + DWORD buffer_max_size, buffer_size; + GstMapInfo map_info; + GstMemory *memory; + BYTE *buf_data; + + if (FAILED(hr = IMFSample_GetBufferByIndex(mf_sample, i, &mf_buffer))) + goto fail; + + if (FAILED(hr = IMFMediaBuffer_GetMaxLength(mf_buffer, &buffer_max_size))) + goto fail; + + if (FAILED(hr = IMFMediaBuffer_GetCurrentLength(mf_buffer, &buffer_size))) + goto fail; + + memory = gst_allocator_alloc(NULL, buffer_size, NULL); + gst_memory_resize(memory, 0, buffer_size); + + if (!(gst_memory_map(memory, &map_info, GST_MAP_WRITE))) + { + hr = E_FAIL; + goto fail; + } + + if (FAILED(hr = IMFMediaBuffer_Lock(mf_buffer, &buf_data, NULL, NULL))) + goto fail; + + memcpy(map_info.data, buf_data, buffer_size); + + if (FAILED(hr = IMFMediaBuffer_Unlock(mf_buffer))) + goto fail; + + if (FAILED(hr = IMFMediaBuffer_SetCurrentLength(mf_buffer, buffer_size))) + goto fail; + + gst_memory_unmap(memory, &map_info); + + gst_buffer_append_memory(out, memory); + + IMFMediaBuffer_Release(mf_buffer); + mf_buffer = NULL; + } + + return out; + +fail: + ERR("Failed to copy IMFSample to GstBuffer, hr = %#x\n", hr); + if (mf_buffer) + IMFMediaBuffer_Release(mf_buffer); + gst_buffer_unref(out); + return NULL; +} -- 2.29.2
On 11/18/20 2:52 PM, Derek Lesho wrote:
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/audioconvert.c | 202 +++++++++++++++++++++++++++++- dlls/winegstreamer/gst_private.h | 1 + dlls/winegstreamer/mfplat.c | 75 +++++++++++ 3 files changed, 273 insertions(+), 5 deletions(-)
diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c index 04227d168ae..deb5ceb2f7c 100644 --- a/dlls/winegstreamer/audioconvert.c +++ b/dlls/winegstreamer/audioconvert.c @@ -24,7 +24,8 @@ struct audio_converter IMFMediaType *input_type; IMFMediaType *output_type; CRITICAL_SECTION cs; - BOOL valid_state; + BOOL valid_state, inflight; + GstElement *container, *appsrc, *audioconvert, *resampler, *appsink; };
static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) @@ -69,6 +70,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface) if (!refcount) { DeleteCriticalSection(&transform->cs); + gst_object_unref(transform->container); heap_free(transform); }
@@ -261,22 +263,54 @@ static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface static void audio_converter_update_pipeline_state(struct audio_converter *converter) { GstCaps *input_caps, *output_caps; + GstStructure *input_structure, *output_structure; + guint64 channels_in, channels_out, channel_mask = 0;
if (!(converter->valid_state = converter->input_type && converter->output_type)) + { + gst_element_set_state(converter->container, GST_STATE_READY); return; + }
if (!(input_caps = caps_from_mf_media_type(converter->input_type))) { converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); return; } if (!(output_caps = caps_from_mf_media_type(converter->output_type))) { converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); + gst_caps_unref(input_caps); + return; + } + + /* audioconvert needs a valid channel-mask */
Well, that's not quite true, at least from my reading of the source, but it does need a valid channel mask for configurations other than mono or stereo. The code below is confusing, though; I'm not sure why channel-mask would be present in the caps at all. Note that a simpler way to set the channel mask is to use gst_audio_info_to_caps() in caps_from_mf_media_type().
+ input_structure = gst_caps_get_structure(input_caps, 0); + output_structure = gst_caps_get_structure(output_caps, 0); + + if (!gst_structure_get(input_structure, "channels", G_TYPE_INT, &channels_in, NULL) || + !gst_structure_get(output_structure, "channels", G_TYPE_INT, &channels_out, NULL)) + { + converter->valid_state = FALSE; + gst_element_set_state(converter->container, GST_STATE_READY); gst_caps_unref(input_caps); + gst_caps_unref(output_caps); return; }
+ gst_structure_get(input_structure, "channel-mask", GST_TYPE_BITMASK, &channel_mask, NULL); + if (channel_mask == 0) + gst_caps_set_simple(input_caps, "channel-mask", GST_TYPE_BITMASK, (guint64) (1 << channels_in) - 1, NULL); + + gst_structure_get(output_structure, "channel-mask", GST_TYPE_BITMASK, &channel_mask, NULL); + if (channel_mask == 0) + gst_caps_set_simple(output_caps, "channel-mask", GST_TYPE_BITMASK, (guint64) (1 << channels_out) - 1, NULL); + + g_object_set(converter->appsrc, "caps", input_caps, NULL); + g_object_set(converter->appsink, "caps", output_caps, NULL); + if (TRACE_ON(mfplat)) { gchar *input_caps_str, *output_caps_str; @@ -293,6 +327,7 @@ static void audio_converter_update_pipeline_state(struct audio_converter *conver
gst_caps_unref(input_caps); gst_caps_unref(output_caps); + gst_element_set_state(converter->container, GST_STATE_PLAYING);
return; } @@ -496,17 +531,114 @@ static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_ME
static HRESULT WINAPI audio_converter_ProcessInput(IMFTransform *iface, DWORD id, IMFSample *sample, DWORD flags) { - FIXME("%p, %u, %p, %#x.\n", iface, id, sample, flags); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + GstBuffer *gst_buffer; + HRESULT hr = S_OK; + int ret;
- return E_NOTIMPL; + TRACE("%p, %u, %p, %#x.\n", iface, id, sample, flags); + + if (flags) + WARN("Unsupported flags %#x\n", flags); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + EnterCriticalSection(&converter->cs); + + if (!converter->valid_state) + { + hr = MF_E_TRANSFORM_TYPE_NOT_SET; + goto done; + } + + if (converter->inflight) + { + hr = MF_E_NOTACCEPTING; + goto done; + } + + if (!(gst_buffer = gst_buffer_from_mf_sample(sample)))
These parentheses are redundant (and I don't think particularly clarifying). The same applies to many other cases in this patch.
+ { + hr = E_FAIL; + goto done; + } + + g_signal_emit_by_name(converter->appsrc, "push-buffer", gst_buffer, &ret); + gst_buffer_unref(gst_buffer); + if (ret != GST_FLOW_OK) + { + ERR("Couldn't push buffer ret = %d\n", ret);
gst_flow_get_name() may be a good idea here.
+ hr = E_FAIL; + goto done; + }
None of these gotos are actually saving you any lines.
+ + converter->inflight = TRUE; + + done: + LeaveCriticalSection(&converter->cs); + + return hr; }
static HRESULT WINAPI audio_converter_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, MFT_OUTPUT_DATA_BUFFER *samples, DWORD *status) { - FIXME("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + MFT_OUTPUT_DATA_BUFFER *relevant_buffer = NULL; + GstSample *sample; + HRESULT hr = S_OK; + unsigned int i;
- return E_NOTIMPL; + TRACE("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + + if (flags) + WARN("Unsupported flags %#x\n", flags); + + for (i = 0; i < count; i++) + { + MFT_OUTPUT_DATA_BUFFER *out_buffer = &samples[i]; + + if (out_buffer->dwStreamID != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (relevant_buffer) + return MF_E_INVALIDSTREAMNUMBER; + + relevant_buffer = out_buffer; + } + + if (!relevant_buffer) + return S_OK;
This code is very strange. Isn't it easier just to validate "count == 1"? And is it really correct to return MF_E_INVALIDSTREAMNUMBER if more than one sample is passed?
+ + EnterCriticalSection(&converter->cs); + + if (!converter->valid_state) + { + hr = MF_E_TRANSFORM_TYPE_NOT_SET; + goto done; + } + + if (!converter->inflight) + { + hr = MF_E_TRANSFORM_NEED_MORE_INPUT; + goto done; + }
More gotos that don't save you anything.
+ + g_signal_emit_by_name(converter->appsink, "pull-sample", &sample); + + converter->inflight = FALSE; + + relevant_buffer->pSample = mf_sample_from_gst_buffer(gst_sample_get_buffer(sample)); + gst_sample_unref(sample); + relevant_buffer->dwStatus = S_OK; + relevant_buffer->pEvents = NULL; + *status = 0; + + done: + LeaveCriticalSection(&converter->cs); + + return hr; }
static const IMFTransformVtbl audio_converter_vtbl = @@ -542,6 +674,7 @@ static const IMFTransformVtbl audio_converter_vtbl = HRESULT audio_converter_create(REFIID riid, void **ret) { struct audio_converter *object; + HRESULT hr;
TRACE("%s %p\n", debugstr_guid(riid), ret);
@@ -553,6 +686,65 @@ HRESULT audio_converter_create(REFIID riid, void **ret)
InitializeCriticalSection(&object->cs);
+ object->container = gst_bin_new(NULL); + + if (!(object->appsrc = gst_element_factory_make("appsrc", NULL))) + { + ERR("Failed to create appsrc");
I'd recommend including a message here about possibly missing gst-plugins-base (and similarly below), since it's a very common mistake for users to make.
+ hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->appsrc); + + if (!(object->audioconvert = gst_element_factory_make("audioconvert", NULL))) + { + ERR("Failed to create converter\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->audioconvert); + + if (!(object->resampler = gst_element_factory_make("audioresample", NULL))) + { + ERR("Failed to create resampler\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->resampler); + + if (!(object->appsink = gst_element_factory_make("appsink", NULL))) + { + ERR("Failed to create appsink\n"); + hr = E_FAIL; + goto failed; + } + gst_bin_add(GST_BIN(object->container), object->appsink); + + if (!(gst_element_link(object->appsrc, object->audioconvert))) + { + ERR("Failed to link appsrc to audioconvert\n"); + hr = E_FAIL; + goto failed; + } + + if (!(gst_element_link(object->audioconvert, object->resampler))) + { + ERR("Failed to link audioconvert to resampler\n"); + hr = E_FAIL; + goto failed; + } + + if (!(gst_element_link(object->resampler, object->appsink))) + { + ERR("Failed to link resampler to appsink\n"); + hr = E_FAIL; + goto failed; + } +
More gotos that don't save you anything.
*ret = &object->IMFTransform_iface; return S_OK; +failed: + + IMFTransform_Release(&object->IMFTransform_iface); + return hr; } diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 7889c996204..7f96c06dfaf 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -81,6 +81,7 @@ HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HI IMFMediaType *mf_media_type_from_caps(const GstCaps *caps) DECLSPEC_HIDDEN; GstCaps *caps_from_mf_media_type(IMFMediaType *type) DECLSPEC_HIDDEN; IMFSample *mf_sample_from_gst_buffer(GstBuffer *in) DECLSPEC_HIDDEN; +GstBuffer *gst_buffer_from_mf_sample(IMFSample *in) DECLSPEC_HIDDEN;
HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN;
diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 909fb8b3572..7fdf722ec2e 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -877,3 +877,78 @@ done:
return out; } + +GstBuffer* gst_buffer_from_mf_sample(IMFSample *mf_sample) +{ + GstBuffer *out = gst_buffer_new(); + IMFMediaBuffer *mf_buffer = NULL; + LONGLONG duration, time; + DWORD buffer_count; + unsigned int i; + HRESULT hr; + + if (FAILED(hr = IMFSample_GetSampleDuration(mf_sample, &duration))) + goto fail; + + if (FAILED(hr = IMFSample_GetSampleTime(mf_sample, &time))) + goto fail; + + GST_BUFFER_DURATION(out) = duration; + GST_BUFFER_PTS(out) = time * 100; + + if (FAILED(hr = IMFSample_GetBufferCount(mf_sample, &buffer_count))) + goto fail; + + for (i = 0; i < buffer_count; i++) + { + DWORD buffer_max_size, buffer_size; + GstMapInfo map_info; + GstMemory *memory; + BYTE *buf_data; + + if (FAILED(hr = IMFSample_GetBufferByIndex(mf_sample, i, &mf_buffer))) + goto fail; + + if (FAILED(hr = IMFMediaBuffer_GetMaxLength(mf_buffer, &buffer_max_size))) + goto fail;
This value is unused.
+ + if (FAILED(hr = IMFMediaBuffer_GetCurrentLength(mf_buffer, &buffer_size))) + goto fail; + + memory = gst_allocator_alloc(NULL, buffer_size, NULL); + gst_memory_resize(memory, 0, buffer_size); + + if (!(gst_memory_map(memory, &map_info, GST_MAP_WRITE))) + { + hr = E_FAIL; + goto fail; + } + + if (FAILED(hr = IMFMediaBuffer_Lock(mf_buffer, &buf_data, NULL, NULL))) + goto fail; + + memcpy(map_info.data, buf_data, buffer_size); + + if (FAILED(hr = IMFMediaBuffer_Unlock(mf_buffer))) + goto fail; + + if (FAILED(hr = IMFMediaBuffer_SetCurrentLength(mf_buffer, buffer_size))) + goto fail;
This doesn't look like it belongs here.
+ + gst_memory_unmap(memory, &map_info); + + gst_buffer_append_memory(out, memory); + + IMFMediaBuffer_Release(mf_buffer); + mf_buffer = NULL; + } + + return out; + +fail: + ERR("Failed to copy IMFSample to GstBuffer, hr = %#x\n", hr); + if (mf_buffer) + IMFMediaBuffer_Release(mf_buffer); + gst_buffer_unref(out); + return NULL; +}
+ /* audioconvert needs a valid channel-mask */ Well, that's not quite true, at least from my reading of the source, but it does need a valid channel mask for configurations other than mono or stereo.
The code below is confusing, though; I'm not sure why channel-mask would be present in the caps at all. Via MF_MT_AUDIO_CHANNEL_MASK.
Note that a simpler way to set the channel mask is to use gst_audio_info_to_caps() in caps_from_mf_media_type(). Mind if I just move this functionality to caps_from_mf_media_type? I remember having problems with gst_audio_info_to_caps.
On 12/1/20 2:54 PM, Derek Lesho wrote:
+ /* audioconvert needs a valid channel-mask */ Well, that's not quite true, at least from my reading of the source, but it does need a valid channel mask for configurations other than mono or stereo.
The code below is confusing, though; I'm not sure why channel-mask would be present in the caps at all. Via MF_MT_AUDIO_CHANNEL_MASK.
Indeed, I missed that, sorry.
Note that a simpler way to set the channel mask is to use gst_audio_info_to_caps() in caps_from_mf_media_type().
Mind if I just move this functionality to caps_from_mf_media_type? I remember having problems with gst_audio_info_to_caps.
I don't think that makes very much sense in general, no. What problems did you have with gst_audio_info_to_caps()?
On 12/1/20 4:27 PM, Zebediah Figura (she/her) wrote:
Note that a simpler way to set the channel mask is to use gst_audio_info_to_caps() in caps_from_mf_media_type(). Mind if I just move this functionality to caps_from_mf_media_type? I remember having problems with gst_audio_info_to_caps.
I don't think that makes very much sense in general, no. What problems did you have with gst_audio_info_to_caps()?
I don't remember unfortunately. I'll give it another shot and report back, although I'm starting to have doubts about whether or not we want channel-mask to be set when MF_MT_CHANNEL_MASK isn't.
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/audioconvert.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c index deb5ceb2f7c..cb7d4a15bf8 100644 --- a/dlls/winegstreamer/audioconvert.c +++ b/dlls/winegstreamer/audioconvert.c @@ -106,16 +106,37 @@ static HRESULT WINAPI audio_converter_GetStreamIDs(IMFTransform *iface, DWORD in static HRESULT WINAPI audio_converter_GetInputStreamInfo(IMFTransform *iface, DWORD id, MFT_INPUT_STREAM_INFO *info) { - FIXME("%p %u %p.\n", iface, id, info); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); - return E_NOTIMPL; + TRACE("%p %u %p\n", converter, id, info); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + info->dwFlags = MFT_INPUT_STREAM_WHOLE_SAMPLES | MFT_INPUT_STREAM_DOES_NOT_ADDREF; + info->cbMaxLookahead = 0; + info->cbAlignment = 0; + info->hnsMaxLatency = 0; + return S_OK; } static HRESULT WINAPI audio_converter_GetOutputStreamInfo(IMFTransform *iface, DWORD id, MFT_OUTPUT_STREAM_INFO *info) { - FIXME("%p %u %p.\n", iface, id, info); + struct audio_converter *converter = impl_audio_converter_from_IMFTransform(iface); + MFT_OUTPUT_STREAM_INFO stream_info = {}; - return E_NOTIMPL; + TRACE("%p %u %p\n", converter, id, info); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + stream_info.dwFlags = MFT_OUTPUT_STREAM_PROVIDES_SAMPLES; + stream_info.cbSize = 0; + stream_info.cbAlignment = 0; + + *info = stream_info; + + return S_OK; } static HRESULT WINAPI audio_converter_GetAttributes(IMFTransform *iface, IMFAttributes **attributes) -- 2.29.2
On 11/18/20 2:52 PM, Derek Lesho wrote:
Serves as a wrapper of audioconvert, and roughly fills the roll of Windows' CLSID_CResamplerMediaObject to convert audio to the format accepted by the streaming audio renderer.
Spelling error: "roll"
Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com> --- v2: Fix intermediate compile errors. --- dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/audioconvert.c | 378 +++++++++++++++++++ dlls/winegstreamer/gst_private.h | 2 + dlls/winegstreamer/mfplat.c | 77 ++++ dlls/winegstreamer/winegstreamer_classes.idl | 6 + 5 files changed, 464 insertions(+) create mode 100644 dlls/winegstreamer/audioconvert.c
diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index e578d194f7f..0b3229160b9 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -6,6 +6,7 @@ EXTRALIBS = $(GSTREAMER_LIBS) $(PTHREAD_LIBS) PARENTSRC = ../strmbase
C_SRCS = \ + audioconvert.c \ filter.c \ gst_cbs.c \ gstdemux.c \ diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c new file mode 100644 index 00000000000..91fa556cb88 --- /dev/null +++ b/dlls/winegstreamer/audioconvert.c @@ -0,0 +1,378 @@ +#include "config.h" + +#include "gst_private.h" + +#include "mfapi.h" +#include "mferror.h" +#include "mfidl.h" + +#include "wine/debug.h" +#include "wine/heap.h" + +WINE_DEFAULT_DEBUG_CHANNEL(mfplat); + +static const GUID *raw_types[] = { + &MFAudioFormat_PCM, + &MFAudioFormat_Float, +}; + +struct audio_converter +{ + IMFTransform IMFTransform_iface; + LONG refcount; +}; + +static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface) +{ + return CONTAINING_RECORD(iface, struct audio_converter, IMFTransform_iface); +} + +static HRESULT WINAPI audio_converter_QueryInterface(IMFTransform *iface, REFIID riid, void **obj) +{ + TRACE("%p, %s, %p.\n", iface, debugstr_guid(riid), obj); + + if (IsEqualIID(riid, &IID_IMFTransform) || + IsEqualIID(riid, &IID_IUnknown)) + { + *obj = iface; + IMFTransform_AddRef(iface); + return S_OK; + } + + WARN("Unsupported %s.\n", debugstr_guid(riid)); + *obj = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI audio_converter_AddRef(IMFTransform *iface) +{ + struct audio_converter *transform = impl_audio_converter_from_IMFTransform(iface); + ULONG refcount = InterlockedIncrement(&transform->refcount); + + TRACE("%p, refcount %u.\n", iface, refcount); + + return refcount; +} + +static ULONG WINAPI audio_converter_Release(IMFTransform *iface) +{ + struct audio_converter *transform = impl_audio_converter_from_IMFTransform(iface); + ULONG refcount = InterlockedDecrement(&transform->refcount); + + TRACE("%p, refcount %u.\n", iface, refcount); + + if (!refcount) + { + heap_free(transform); + } + + return refcount; +} + +static HRESULT WINAPI audio_converter_GetStreamLimits(IMFTransform *iface, DWORD *input_minimum, DWORD *input_maximum, + DWORD *output_minimum, DWORD *output_maximum) +{ + TRACE("%p, %p, %p, %p, %p.\n", iface, input_minimum, input_maximum, output_minimum, output_maximum); + + *input_minimum = *input_maximum = *output_minimum = *output_maximum = 1; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetStreamCount(IMFTransform *iface, DWORD *inputs, DWORD *outputs) +{ + TRACE("%p, %p, %p.\n", iface, inputs, outputs); + + *inputs = *outputs = 1; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetStreamIDs(IMFTransform *iface, DWORD input_size, DWORD *inputs, + DWORD output_size, DWORD *outputs) +{ + TRACE("%p %u %p %u %p.\n", iface, input_size, inputs, output_size, outputs); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStreamInfo(IMFTransform *iface, DWORD id, MFT_INPUT_STREAM_INFO *info) +{ + FIXME("%p %u %p.\n", iface, id, info); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStreamInfo(IMFTransform *iface, DWORD id, MFT_OUTPUT_STREAM_INFO *info) +{ + FIXME("%p %u %p.\n", iface, id, info); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetAttributes(IMFTransform *iface, IMFAttributes **attributes) +{ + FIXME("%p, %p.\n", iface, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStreamAttributes(IMFTransform *iface, DWORD id, + IMFAttributes **attributes) +{ + FIXME("%p, %u, %p.\n", iface, id, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStreamAttributes(IMFTransform *iface, DWORD id, + IMFAttributes **attributes) +{ + FIXME("%p, %u, %p.\n", iface, id, attributes); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_DeleteInputStream(IMFTransform *iface, DWORD id) +{ + TRACE("%p, %u.\n", iface, id); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_AddInputStreams(IMFTransform *iface, DWORD streams, DWORD *ids) +{ + TRACE("%p, %u, %p.\n", iface, streams, ids); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputAvailableType(IMFTransform *iface, DWORD id, DWORD index, + IMFMediaType **type) +{ + IMFMediaType *ret; + HRESULT hr; + + TRACE("%p, %u, %u, %p.\n", iface, id, index, type); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (index >= ARRAY_SIZE(raw_types)) + return MF_E_NO_MORE_TYPES; + + if (FAILED(hr = MFCreateMediaType(&ret))) + return hr; + + if (FAILED(hr = IMFMediaType_SetGUID(ret, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio))) + { + IMFMediaType_Release(ret); + return hr; + } + + if (FAILED(hr = IMFMediaType_SetGUID(ret, &MF_MT_SUBTYPE, raw_types[index]))) + { + IMFMediaType_Release(ret); + return hr; + } + + *type = ret; + + return S_OK; +} + +static HRESULT WINAPI audio_converter_GetOutputAvailableType(IMFTransform *iface, DWORD id, DWORD index, + IMFMediaType **type) +{ + IMFMediaType *output_type; + HRESULT hr; + + static const DWORD rates[] = {44100, 48000}; + static const DWORD channel_cnts[] = {1, 2, 6}; + static const DWORD sizes[] = {16, 24, 32}; + const GUID *subtype; + DWORD rate, channels, bps; + + TRACE("%p, %u, %u, %p.\n", iface, id, index, type); + + if (id != 0) + return MF_E_INVALIDSTREAMNUMBER; + + if (index >= (2/*rates*/ * 3/*layouts*/ * 3/*bps PCM*/) + (2 * 3)) + return MF_E_NO_MORE_TYPES; + + if (FAILED(hr = MFCreateMediaType(&output_type))) + return hr; + + if (index < 2 * 3 * 3) + { + subtype = &MFAudioFormat_PCM; + rate = rates[index % 2]; + channels = channel_cnts[(index / 2) % 3]; + bps = sizes[(index / (2*3)) % 3]; + } + else + { + index -= (2 * 3 * 3); + subtype = &MFAudioFormat_Float; + bps = 32; + rate = rates[index % 2]; + channels = channel_cnts[(index / 2) % 3]; + } +
This is mildly awkward; perhaps it would be better to replace "sizes" with something like static const struct { const GUID *subtype; DWORD depth; } formats[] = { {&MFAudioFormat_PCM, 16}, {&MFAudioFormat_PCM, 24}, {&MFAudioFormat_PCM, 32}, {&MFAudioFormat_Float, 32}, };
+ + if (FAILED(hr = IMFMediaType_SetGUID(output_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio))) + goto fail; + if (FAILED(hr = IMFMediaType_SetGUID(output_type, &MF_MT_SUBTYPE, subtype))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, rate))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_NUM_CHANNELS, channels))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, bps))) + goto fail; + + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_BLOCK_ALIGNMENT, channels * bps / 8))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_AVG_BYTES_PER_SECOND, rate * channels * bps / 8))) + goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_AUDIO_CHANNEL_MASK, + channels == 1 ? SPEAKER_FRONT_CENTER : + channels == 2 ? SPEAKER_FRONT_LEFT | SPEAKER_FRONT_RIGHT : + /*channels == 6*/ 0x3F)))
These are KSAUDIO_SPEAKER_MONO, KSAUDIO_SPEAKER_STEREO, and KSAUDIO_SPEAKER_5POINT1 respectively.
+ goto fail; + if (FAILED(hr = IMFMediaType_SetUINT32(output_type, &MF_MT_ALL_SAMPLES_INDEPENDENT, TRUE))) + goto fail; + + *type = output_type; + + return S_OK; + fail:
Please don't insert tabs before labels; it makes them look like regular statements.
+ IMFMediaType_Release(output_type); + return hr; +} +
I'm not sure why this function (and some of the others here) weren't split out into a separate patch like e.g. SetInputType(). Personally I'd recommend splitting out the implementations of all functions.
+static HRESULT WINAPI audio_converter_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_SetOutputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) +{ + FIXME("%p, %u, %p.\n", iface, id, type); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputCurrentType(IMFTransform *iface, DWORD id, IMFMediaType **type) +{ + FIXME("%p, %u, %p.\n", iface, id, type); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetInputStatus(IMFTransform *iface, DWORD id, DWORD *flags) +{ + FIXME("%p, %u, %p.\n", iface, id, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_GetOutputStatus(IMFTransform *iface, DWORD *flags) +{ + FIXME("%p, %p.\n", iface, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_SetOutputBounds(IMFTransform *iface, LONGLONG lower, LONGLONG upper) +{ + FIXME("%p, %s, %s.\n", iface, wine_dbgstr_longlong(lower), wine_dbgstr_longlong(upper)); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessEvent(IMFTransform *iface, DWORD id, IMFMediaEvent *event) +{ + TRACE("%p, %u, %p.\n", iface, id, event); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +}
Why S_OK?
+ +static HRESULT WINAPI audio_converter_ProcessInput(IMFTransform *iface, DWORD id, IMFSample *sample, DWORD flags) +{ + FIXME("%p, %u, %p, %#x.\n", iface, id, sample, flags); + + return E_NOTIMPL; +} + +static HRESULT WINAPI audio_converter_ProcessOutput(IMFTransform *iface, DWORD flags, DWORD count, + MFT_OUTPUT_DATA_BUFFER *samples, DWORD *status) +{ + FIXME("%p, %#x, %u, %p, %p.\n", iface, flags, count, samples, status); + + return E_NOTIMPL; +} + +static const IMFTransformVtbl audio_converter_vtbl = +{ + audio_converter_QueryInterface, + audio_converter_AddRef, + audio_converter_Release, + audio_converter_GetStreamLimits, + audio_converter_GetStreamCount, + audio_converter_GetStreamIDs, + audio_converter_GetInputStreamInfo, + audio_converter_GetOutputStreamInfo, + audio_converter_GetAttributes, + audio_converter_GetInputStreamAttributes, + audio_converter_GetOutputStreamAttributes, + audio_converter_DeleteInputStream, + audio_converter_AddInputStreams, + audio_converter_GetInputAvailableType, + audio_converter_GetOutputAvailableType, + audio_converter_SetInputType, + audio_converter_SetOutputType, + audio_converter_GetInputCurrentType, + audio_converter_GetOutputCurrentType, + audio_converter_GetInputStatus, + audio_converter_GetOutputStatus, + audio_converter_SetOutputBounds, + audio_converter_ProcessEvent, + audio_converter_ProcessMessage, + audio_converter_ProcessInput, + audio_converter_ProcessOutput, +}; + +HRESULT audio_converter_create(REFIID riid, void **ret) +{ + struct audio_converter *object; + HRESULT hr; + + TRACE("%s %p\n", debugstr_guid(riid), ret); + + if (!(object = heap_alloc_zero(sizeof(*object)))) + return E_OUTOFMEMORY; + + object->IMFTransform_iface.lpVtbl = &audio_converter_vtbl; + object->refcount = 1; + + *ret = &object->IMFTransform_iface; + return S_OK; +} diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 28e424439d8..7889c996204 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -84,4 +84,6 @@ IMFSample *mf_sample_from_gst_buffer(GstBuffer *in) DECLSPEC_HIDDEN;
HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN;
+HRESULT audio_converter_create(REFIID riid, void **ret) DECLSPEC_HIDDEN; + #endif /* __GST_PRIVATE_INCLUDED__ */ diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c index 3d224a5accc..909fb8b3572 100644 --- a/dlls/winegstreamer/mfplat.c +++ b/dlls/winegstreamer/mfplat.c @@ -405,6 +405,8 @@ failed:
static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618, 0x5e5a, 0x468a, {0x9f, 0x15, 0xd8, 0x27, 0xa9, 0xa0, 0x81, 0x62}};
+static GUID CLSID_WINEAudioConverter = {0x6a170414,0xaad9,0x4693,{0xb8,0x06,0x3a,0x0c,0x47,0xc5,0x70,0xd6}}; +
Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). Also, this can be const.
static const struct class_object { const GUID *clsid; @@ -414,6 +416,7 @@ class_objects[] = { { &CLSID_VideoProcessorMFT, &video_processor_create }, { &CLSID_GStreamerByteStreamHandler, &winegstreamer_stream_handler_create }, + { &CLSID_WINEAudioConverter, &audio_converter_create }, };
HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj) @@ -442,6 +445,80 @@ HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj) return CLASS_E_CLASSNOTAVAILABLE; }
+static WCHAR audio_converterW[] = {'A','u','d','i','o',' ','C','o','n','v','e','r','t','e','r',0}; +const GUID *audio_converter_supported_types[] =
Missing "static" here.
+{ + &MFAudioFormat_PCM, + &MFAudioFormat_Float, +}; + +static const struct mft +{ + const GUID *clsid; + const GUID *category; + LPWSTR name; + const UINT32 flags; + const GUID *major_type; + const UINT32 input_types_count; + const GUID **input_types; + const UINT32 output_types_count; + const GUID **output_types; + IMFAttributes *attributes; +} +mfts[] = +{ + { + &CLSID_WINEAudioConverter, + &MFT_CATEGORY_AUDIO_EFFECT, + audio_converterW, + MFT_ENUM_FLAG_SYNCMFT, + &MFMediaType_Audio, + ARRAY_SIZE(audio_converter_supported_types), + audio_converter_supported_types, + ARRAY_SIZE(audio_converter_supported_types), + audio_converter_supported_types, + NULL + }, +}; + +HRESULT mfplat_DllRegisterServer(void) +{ + unsigned int i; + HRESULT hr; + + for (i = 0; i < ARRAY_SIZE(mfts); i++) + { + const struct mft *cur = &mfts[i]; + + MFT_REGISTER_TYPE_INFO *input_types, *output_types; + input_types = heap_alloc(cur->input_types_count * sizeof(input_types[0])); + output_types = heap_alloc(cur->output_types_count * sizeof(output_types[0]));
This is some confusing spacing. I expect either a blank line after all declarations or no blank line at all, but this makes me think "wait, where's input_types declared?" Separately, I think it'd be better just to use MFT_REGISTER_TYPE_INFO directly in your "struct mft", and avoid allocation. It'd have to be non-const (or duplicate GUID definitions), but I think that's less awkward on the whole.
+ for (i = 0; i < cur->input_types_count; i++) + { + input_types[i].guidMajorType = *(cur->major_type); + input_types[i].guidSubtype = *(cur->input_types[i]); + } + for (i = 0; i < cur->output_types_count; i++) + { + output_types[i].guidMajorType = *(cur->major_type); + output_types[i].guidSubtype = *(cur->output_types[i]); + } + + hr = MFTRegister(*(cur->clsid), *(cur->category), cur->name, cur->flags, cur->input_types_count, + input_types, cur->output_types_count, output_types, cur->attributes); + + heap_free(input_types); + heap_free(output_types); + + if (FAILED(hr)) + { + FIXME("Failed to register MFT, hr %#x\n", hr); + return hr; + } + } + return S_OK; +} +
This registration could also be a separate patch.
static const struct { const GUID *subtype; diff --git a/dlls/winegstreamer/winegstreamer_classes.idl b/dlls/winegstreamer/winegstreamer_classes.idl index 1dc4ba9a10b..cf1fc69f38a 100644 --- a/dlls/winegstreamer/winegstreamer_classes.idl +++ b/dlls/winegstreamer/winegstreamer_classes.idl @@ -61,3 +61,9 @@ coclass VideoProcessorMFT {} uuid(317df618-5e5a-468a-9f15-d827a9a08162) ] coclass GStreamerByteStreamHandler {} + +[ + threading(both), + uuid(6a170414-aad9-4693-b806-3a0c47c570d6) +] +coclass WINEAudioConverter { }
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead.
Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific.
Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion?
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems.
Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine. Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source). In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
On 12/2/20 3:07 PM, Derek Lesho wrote:
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems.
Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine.
I'm not sure I understand; what do you mean by this?
Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source).
It would be nice to confirm whether this is the case.
In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
On 12/2/20 4:31 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:07 PM, Derek Lesho wrote:
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) +{ + FIXME("%p, %u.\n", iface, message); + + return S_OK; +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems. Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine. I'm not sure I understand; what do you mean by this? The SAR on wine, at-least on my system, only accepts one input type (the one derived from IAudioClient::GetMixFormat). So we need to be able to convert the one media type we are allowed to use from the source to the one media type the SAR supports. Naturally, you end up with cases where one is floating point and the other is PCM.
Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source). It would be nice to confirm whether this is the case. At-least for the MFTs I've seen so far, it is the case. (both the AAC decoder and WMA decoder guarantee a floating point and PCM output type)
In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
On 12/2/20 3:37 PM, Derek Lesho wrote:
On 12/2/20 4:31 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:07 PM, Derek Lesho wrote:
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
> +static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform > *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) > +{ > + FIXME("%p, %u.\n", iface, message); > + > + return S_OK; > +} Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems. Why not CLSID_CResamplerMediaObject? In fact, we already have one application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine. I'm not sure I understand; what do you mean by this? The SAR on wine, at-least on my system, only accepts one input type (the one derived from IAudioClient::GetMixFormat). So we need to be able to convert the one media type we are allowed to use from the source to the one media type the SAR supports. Naturally, you end up with cases where one is floating point and the other is PCM.
Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source). It would be nice to confirm whether this is the case. At-least for the MFTs I've seen so far, it is the case. (both the AAC decoder and WMA decoder guarantee a floating point and PCM output type)
In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
Okay, that's unfortunate, but makes sense. And there's no transform present on Windows which can do simple integer/float PCM conversion?
On 12/2/20 4:47 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:37 PM, Derek Lesho wrote:
On 12/2/20 4:31 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:07 PM, Derek Lesho wrote:
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote: >> +static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform >> *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) >> +{ >> + FIXME("%p, %u.\n", iface, message); >> + >> + return S_OK; >> +} > Why S_OK? Because the media session sends some messages to the transform such as MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't returned. I take it you'd like me to actually implement this method instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems. > Why not CLSID_CResamplerMediaObject? In fact, we already have one > application that needs it (bug 47781). The only reason I didn't expose it as CLSID_CResamplerMediaObject is that I didn't want to imply that I was basing the interface and types supported off of that specific object. For instance, that object, from what I can see on the MSDN, doesn't support PCM<->Float conversions, and vice versa. Is this not a big enough deal to keep it separate?
Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine. I'm not sure I understand; what do you mean by this? The SAR on wine, at-least on my system, only accepts one input type (the one derived from IAudioClient::GetMixFormat). So we need to be able to convert the one media type we are allowed to use from the source to the one media type the SAR supports. Naturally, you end up with cases where one is floating point and the other is PCM. Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source). It would be nice to confirm whether this is the case. At-least for the MFTs I've seen so far, it is the case. (both the AAC decoder and WMA decoder guarantee a floating point and PCM output type) In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
Okay, that's unfortunate, but makes sense. And there's no transform present on Windows which can do simple integer/float PCM conversion? Not that I know of, and even if there was, the topology loader can only hook up one converter automatically. Is this enough of a reason to stay with a custom object GUID for now?
On 12/2/20 3:49 PM, Derek Lesho wrote:
On 12/2/20 4:47 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:37 PM, Derek Lesho wrote:
On 12/2/20 4:31 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 3:07 PM, Derek Lesho wrote:
On 12/2/20 4:01 PM, Zebediah Figura (she/her) wrote:
On 12/1/20 2:06 PM, Derek Lesho wrote: > On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote: >>> +static HRESULT WINAPI audio_converter_ProcessMessage(IMFTransform >>> *iface, MFT_MESSAGE_TYPE message, ULONG_PTR param) >>> +{ >>> + FIXME("%p, %u.\n", iface, message); >>> + >>> + return S_OK; >>> +} >> Why S_OK? > Because the media session sends some messages to the transform > such as > MFT_MESSAGE_NOTIFY_BEGIN_STREAMING and fails to play if S_OK isn't > returned. I take it you'd like me to actually implement this method > instead. Not necessarily, but if nothing needs to be done, then presumably there shouldn't be a FIXME either, and it's not obvious to me what needs to be done. Sorry, I guess that comment should have been more specific. FWIW I've already sent a patch "implementing" this function in v3? Should I remove it and just switch to a trace or just keep it as is? I think it may be better to keep it as is, since some messages not being implemented may cause real problems. >> Why not CLSID_CResamplerMediaObject? In fact, we already have one >> application that needs it (bug 47781). > The only reason I didn't expose it as CLSID_CResamplerMediaObject is > that I didn't want to imply that I was basing the interface and > types > supported off of that specific object. For instance, that object, > from > what I can see on the MSDN, doesn't support PCM<->Float > conversions, and > vice versa. Is this not a big enough deal to keep it separate? > Do we need to support integer/float conversion? Yes, from what I've seen the SAR usually only supports one or the other, at-least on wine. I'm not sure I understand; what do you mean by this? The SAR on wine, at-least on my system, only accepts one input type (the one derived from IAudioClient::GetMixFormat). So we need to be able to convert the one media type we are allowed to use from the source to the one media type the SAR supports. Naturally, you end up with cases where one is floating point and the other is PCM. Exposing a PCM and Float type from the source doesn't solve this either, as the application doesn't have to specify ENUMERATE_SOURCE_TYPES when loading the topology, and in practice, rarely does, so the topology loader is forced to work with whatever the default type happens to be. It may be that on windows the decoders almost always expose both a PCM and floating point type, but currently, we are not using decoder MFTs in wine (and instead decoding in the media source). It would be nice to confirm whether this is the case. At-least for the MFTs I've seen so far, it is the case. (both the AAC decoder and WMA decoder guarantee a floating point and PCM output type) In practice, what this means is that without PCM<->Floating Point capabilities, Borderlands 3 doesn't work.
Okay, that's unfortunate, but makes sense. And there's no transform present on Windows which can do simple integer/float PCM conversion? Not that I know of, and even if there was, the topology loader can only hook up one converter automatically. Is this enough of a reason to stay with a custom object GUID for now?
It seems that way to me, yes.
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+HRESULT mfplat_DllRegisterServer(void) +{ + unsigned int i; + HRESULT hr; + + for (i = 0; i < ARRAY_SIZE(mfts); i++) + { + const struct mft *cur = &mfts[i]; + + MFT_REGISTER_TYPE_INFO *input_types, *output_types; + input_types = heap_alloc(cur->input_types_count * sizeof(input_types[0])); + output_types = heap_alloc(cur->output_types_count * sizeof(output_types[0])); Separately, I think it'd be better just to use MFT_REGISTER_TYPE_INFO directly in your "struct mft", and avoid allocation. It'd have to be non-const (or duplicate GUID definitions), but I think that's less awkward on the whole.
I'm trying to implement this now, but I'm having trouble with it. MFT_REGISTER_TYPE_INFO consists of GUID value fields, not GUID pointer fields. I'm not sure if there's a clean way to fill GUID fields of a struct in an initialization list, even when the array is not const. I definitely don't want to duplicate the GUID definitions, that would be ugly. Also, I'm not sure I necessarily I agree this solution is better than just having some heap allocations in DllRegisterServer, it's not in what I would call a hotpath.
On 12/1/20 3:57 PM, Derek Lesho wrote:
On 12/1/20 3:00 PM, Zebediah Figura (she/her) wrote:
+HRESULT mfplat_DllRegisterServer(void) +{ + unsigned int i; + HRESULT hr; + + for (i = 0; i < ARRAY_SIZE(mfts); i++) + { + const struct mft *cur = &mfts[i]; + + MFT_REGISTER_TYPE_INFO *input_types, *output_types; + input_types = heap_alloc(cur->input_types_count * sizeof(input_types[0])); + output_types = heap_alloc(cur->output_types_count * sizeof(output_types[0])); Separately, I think it'd be better just to use MFT_REGISTER_TYPE_INFO directly in your "struct mft", and avoid allocation. It'd have to be non-const (or duplicate GUID definitions), but I think that's less awkward on the whole.
I'm trying to implement this now, but I'm having trouble with it. MFT_REGISTER_TYPE_INFO consists of GUID value fields, not GUID pointer fields. I'm not sure if there's a clean way to fill GUID fields of a struct in an initialization list, even when the array is not const. I definitely don't want to duplicate the GUID definitions, that would be ugly. Also, I'm not sure I necessarily I agree this solution is better than just having some heap allocations in DllRegisterServer, it's not in what I would call a hotpath.
Performance isn't really the concern—it's more code, and you really should be handling allocation failure. Another solution could be to just use a fixed-size stack buffer.
On 12/2/20 12:07 PM, Zebediah Figura (she/her) wrote:
Performance isn't really the concern—it's more code, and you really should be handling allocation failure.
Another solution could be to just use a fixed-size stack buffer. Okay, I guess I'll do that. There's no limit to how many types you can register for the MFT so I'll just make a max of 100 for both input and output (so ~3.2kb of stack space).
On 12/2/20 11:15 AM, Derek Lesho wrote:
On 12/2/20 12:07 PM, Zebediah Figura (she/her) wrote:
Performance isn't really the concern—it's more code, and you really should be handling allocation failure.
Another solution could be to just use a fixed-size stack buffer. Okay, I guess I'll do that. There's no limit to how many types you can register for the MFT so I'll just make a max of 100 for both input and output (so ~3.2kb of stack space).
I think there's no need for that; just allocate as much stack space as you actually need.
On 12/2/20 12:25 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 11:15 AM, Derek Lesho wrote:
On 12/2/20 12:07 PM, Zebediah Figura (she/her) wrote:
Performance isn't really the concern—it's more code, and you really should be handling allocation failure.
Another solution could be to just use a fixed-size stack buffer. Okay, I guess I'll do that. There's no limit to how many types you can register for the MFT so I'll just make a max of 100 for both input and output (so ~3.2kb of stack space). I think there's no need for that; just allocate as much stack space as you actually need. That varies on a per MFT registration basis, some MFTs require more registration structs than others, since they have more input/output types. Unless you're suggesting I use a VLA?
On 12/2/20 11:26 AM, Derek Lesho wrote:
On 12/2/20 12:25 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 11:15 AM, Derek Lesho wrote:
On 12/2/20 12:07 PM, Zebediah Figura (she/her) wrote:
Performance isn't really the concern—it's more code, and you really should be handling allocation failure.
Another solution could be to just use a fixed-size stack buffer. Okay, I guess I'll do that. There's no limit to how many types you can register for the MFT so I'll just make a max of 100 for both input and output (so ~3.2kb of stack space). I think there's no need for that; just allocate as much stack space as you actually need. That varies on a per MFT registration basis, some MFTs require more registration structs than others, since they have more input/output types. Unless you're suggesting I use a VLA?
No, simply use the maximum array size that any individual transform needs.
On 12/2/20 12:55 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 11:26 AM, Derek Lesho wrote:
On 12/2/20 12:25 PM, Zebediah Figura (she/her) wrote:
On 12/2/20 11:15 AM, Derek Lesho wrote:
On 12/2/20 12:07 PM, Zebediah Figura (she/her) wrote:
Performance isn't really the concern—it's more code, and you really should be handling allocation failure.
Another solution could be to just use a fixed-size stack buffer. Okay, I guess I'll do that. There's no limit to how many types you can register for the MFT so I'll just make a max of 100 for both input and output (so ~3.2kb of stack space). I think there's no need for that; just allocate as much stack space as you actually need. That varies on a per MFT registration basis, some MFTs require more registration structs than others, since they have more input/output types. Unless you're suggesting I use a VLA? No, simply use the maximum array size that any individual transform needs. Not looking forward to the bugs that show up from somebody adding more entries to the supported types list and not increasing the array size just so we can avoid two heap_allocs, but ok.
participants (2)
-
Derek Lesho -
Zebediah Figura (she/her)