Re: [PATCH 4/5] winegstreamer: Implement ::GetInputAvailableType for decode transform.

[Zebediah Figura (she/her)]

On 3/10/21 1:33 PM, Derek Lesho wrote:
> Signed-off-by: Derek Lesho
>
> dlls/winegstreamer/decode_transform.c | 60 +++++++++++++++++++++++++--
>  dlls/winegstreamer/gst_private.h      |  6 ++-
>  dlls/winegstreamer/mfplat.c           |  7 +++-
>  3 files changed, 67 insertions(+), 6 deletions(-)

Applications are creating this specific object, right? Can they be
mentioned in the patch subject or in a commment?

Also, should we add tests, at least corresponding to what the
applications are doing?
> diff --git a/dlls/winegstreamer/decode_transform.c b/dlls/winegstreamer/decode_transform.c
> index f5d4763bde4..55a0c1c6c9b 100644
> --- a/dlls/winegstreamer/decode_transform.c
> +++ b/dlls/winegstreamer/decode_transform.c
> @@ -29,10 +29,33 @@
>
> WINE_DEFAULT_DEBUG_CHANNEL(mfplat);
>
> +const GUID *h264_input_types[] = {&MFVideoFormat_H264};
> +/* NV12 comes first https://docs.microsoft.com/en-us/windows/win32/medfound/mft-decoder-expose-o... . thanks to @vitorhnn */
> +const GUID *h264_output_types[] = {&MFVideoFormat_NV12, &MFVideoFormat_I420, &MFVideoFormat_IYUV, &MFVideoFormat_YUY2, &MFVideoFormat_YV12};

"static const GUID *const h264_input_types[]"
> -
>
> +static struct decoder_desc

"static const", but...
> +{
> - const GUID *major_type;
> - const GUID **input_types;
> - unsigned int input_types_count;
> - const GUID **output_types;
> - unsigned int output_types_count;
>
> +} decoder_descs[] =
> +{
> - { /* DECODER_TYPE_H264 */
> -
>    &MFMediaType_Video,
> -
>    h264_input_types,
> -
>    ARRAY_SIZE(h264_input_types),
> -
>    h264_output_types,
> -
>    ARRAY_SIZE(h264_output_types),
> - },
>
> +};
> -
>
> struct mf_decoder
>  {
>      IMFTransform IMFTransform_iface;
>      LONG refcount;
> - enum decoder_type type;
>
> };

...you might find it cleaner to pass and store a "const struct
decoder_desc *", which would get rid of the need to keep the
"decoder_descs" array in the correct order.
> static struct mf_decoder *impl_mf_decoder_from_IMFTransform(IMFTransform *iface)
> @@ -163,9 +186,36 @@ static HRESULT WINAPI mf_decoder_AddInputStreams(IMFTransform *iface, DWORD stre
>  static HRESULT WINAPI mf_decoder_GetInputAvailableType(IMFTransform *iface, DWORD id, DWORD index,
>          IMFMediaType **type)
>  {
> - FIXME("%p, %u, %u, %p.\n", iface, id, index, type);
> - struct mf_decoder *decoder = impl_mf_decoder_from_IMFTransform(iface);
> - IMFMediaType *input_type;
> - HRESULT hr;
> - return E_NOTIMPL;
> - TRACE("%p, %u, %u, %p\n", decoder, id, index, type);
> -
> - if (id != 0)
> -
>    return MF_E_INVALIDSTREAMNUMBER;
> -
> - if (index >= decoder_descs[decoder->type].input_types_count)
> -
>    return MF_E_NO_MORE_TYPES;
> -
> - if (FAILED(hr = MFCreateMediaType(&input_type)))
> -
>    return hr;
> -
> - if (FAILED(hr = IMFMediaType_SetGUID(input_type, &MF_MT_MAJOR_TYPE, decoder_descs[decoder->type].major_type)))
> - {
> -
>    IMFMediaType_Release(input_type);
> -
>    return hr;
> - }
> -
> - if (FAILED(hr = IMFMediaType_SetGUID(input_type, &MF_MT_SUBTYPE, decoder_descs[decoder->type].input_types[index])))
> - {
> -
>    IMFMediaType_Release(input_type);
> -
>    return hr;
> - }
> -
> - *type = input_type;
> -
> - return S_OK;
>
> }
>
> static HRESULT WINAPI mf_decoder_GetOutputAvailableType(IMFTransform *iface, DWORD id, DWORD index,
> @@ -284,11 +334,11 @@ static const IMFTransformVtbl mf_decoder_vtbl =
>      mf_decoder_ProcessOutput,
>  };
>
> -HRESULT decode_transform_create(REFIID riid, void **obj)
> +HRESULT decode_transform_create(REFIID riid, void **obj, enum decoder_type type)
>  {
>      struct mf_decoder *object;
> - TRACE("%s, %p.\n", debugstr_guid(riid), obj);
> -
>
> TRACE("%s, %p %u.\n", debugstr_guid(riid), obj, type);
>
> if (!(object = heap_alloc_zero(sizeof(*object))))
>     return E_OUTOFMEMORY;
>
> @@ -296,6 +346,8 @@ HRESULT decode_transform_create(REFIID riid, void **obj)
>      object->IMFTransform_iface.lpVtbl = &mf_decoder_vtbl;
>      object->refcount = 1;
> - object->type = type;
> - *obj = &object->IMFTransform_iface;
>    return S_OK;
>
> }
> diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h
> index c97c874fd68..54aba7a09ae 100644
> --- a/dlls/winegstreamer/gst_private.h
> +++ b/dlls/winegstreamer/gst_private.h
> @@ -216,6 +216,10 @@ HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HI
>
> HRESULT audio_converter_create(REFIID riid, void **ret) DECLSPEC_HIDDEN;
>
> -HRESULT decode_transform_create(REFIID riid, void **obj) DECLSPEC_HIDDEN;
> +enum decoder_type
> +{
> - DECODER_TYPE_H264,
>
> +};
> +HRESULT decode_transform_create(REFIID riid, void **obj, enum decoder_type) DECLSPEC_HIDDEN;
>
> #endif /* __GST_PRIVATE_INCLUDED__ */
> diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c
> index cb862375276..65c55e4835d 100644
> --- a/dlls/winegstreamer/mfplat.c
> +++ b/dlls/winegstreamer/mfplat.c
> @@ -402,6 +402,11 @@ static const GUID CLSID_GStreamerByteStreamHandler = {0x317df618, 0x5e5a, 0x468a
>
> static const GUID CLSID_WINEAudioConverter = {0x6a170414,0xaad9,0x4693,{0xb8,0x06,0x3a,0x0c,0x47,0xc5,0x70,0xd6}};
>
> +static HRESULT h264_decoder_create(REFIID riid, void **ret)
> +{
> - return decode_transform_create(riid, ret, DECODER_TYPE_H264);
>
> +}
> -
>
> static const struct class_object
>  {
>      const GUID *clsid;
> @@ -412,7 +417,7 @@ class_objects[] =
>      { &CLSID_VideoProcessorMFT, &video_processor_create },
>      { &CLSID_GStreamerByteStreamHandler, &winegstreamer_stream_handler_create },
>      { &CLSID_WINEAudioConverter, &audio_converter_create },
> - { &CLSID_CMSH264DecoderMFT, &decode_transform_create },
> - { &CLSID_CMSH264DecoderMFT, &h264_decoder_create },
>
> };
>
> HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj)