[PATCH v2 1/4] winegstreamer: Introduce audio conversion transform.

Zebediah Figura (she/her) zfigura at codeweavers.com
Tue Dec 1 14:00:03 CST 2020


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 at 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 { }
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_0x0D9D358A07A17840.asc
Type: application/pgp-keys
Size: 1769 bytes
Desc: not available
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201201/099d5cae/attachment-0001.key>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201201/099d5cae/attachment-0001.sig>


More information about the wine-devel mailing list