Re: [PATCH v3 07/12] winegstreamer: Implement ::SetInputType for audio conversion transform.

[Zebediah Figura (she/her)]

On 12/2/20 1:54 PM, Derek Lesho wrote:
> Signed-off-by: Derek Lesho
> v3: Factor out update_pipeline_state.
>
> dlls/winegstreamer/audioconvert.c | 70 ++++++++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 2 deletions(-)
>
> diff --git a/dlls/winegstreamer/audioconvert.c b/dlls/winegstreamer/audioconvert.c
> index 1174f9df0db..82756281aa9 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"
>
> @@ -17,6 +18,8 @@ struct audio_converter
>  {
>      IMFTransform IMFTransform_iface;
>      LONG refcount;
> - IMFMediaType *input_type;
> - CRITICAL_SECTION cs;
>
> };
>
> static struct audio_converter *impl_audio_converter_from_IMFTransform(IMFTransform *iface)
> @@ -60,6 +63,7 @@ static ULONG WINAPI audio_converter_Release(IMFTransform *iface)
>  if (!refcount)
>  {
> -
>    DeleteCriticalSection(&transform->cs);
>    heap_free(transform);
> }

You also need to reset the CS debug info to 0 before destroying it.
> @@ -251,9 +255,68 @@ fail:
>
> static HRESULT WINAPI audio_converter_SetInputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags)
>  {
> - FIXME("%p, %u, %p, %#x.\n", iface, id, type, flags);
> - GstCaps *input_caps;
> - 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;
> -
>    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;
> -
> -
>    if (!IsEqualGUID(&subtype, &MFAudioFormat_PCM) && !IsEqualGUID(&subtype, &MFAudioFormat_Float))
> -
>        return MF_E_INVALIDTYPE;
> -
> -
>    if (!(input_caps = caps_from_mf_media_type(type)))
> -
>        return MF_E_INVALIDTYPE;
> -
> -
>    gst_caps_unref(input_caps);
> - }
> -
> - if (flags & MFT_SET_TYPE_TEST_ONLY)
> -
>    return S_OK;
> -
> - EnterCriticalSection(&converter->cs);
> -
> - hr = S_OK;
> -
> - if (type)
> - {
> -
>    if (!converter->input_type)
> -
>        hr = MFCreateMediaType(&converter->input_type);
> -
> -
>    if (SUCCEEDED(hr))
> -
>        hr = IMFMediaType_CopyAllItems(type, (IMFAttributes *) converter->input_type);
> - }
> - if (converter->input_type && (!type || FAILED(hr)))

This condition feels more than a little confusing; I don't think it's
worth it just to save a couple lines.
> - {
> -
>    IMFMediaType_Release(converter->input_type);
> -
>    converter->input_type = NULL;
> - }
> -
> - LeaveCriticalSection(&converter->cs);
> -
> - return hr;
>
> }
>
> static HRESULT WINAPI audio_converter_SetOutputType(IMFTransform *iface, DWORD id, IMFMediaType *type, DWORD flags)
> @@ -376,6 +439,9 @@ HRESULT audio_converter_create(REFIID riid, void **ret)
>      object->IMFTransform_iface.lpVtbl = &audio_converter_vtbl;
>      object->refcount = 1;
> - InitializeCriticalSection(&object->cs);
> - object->cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": audio_converter_lock");
> - *ret = &object->IMFTransform_iface;
>    return S_OK;
>
> }