[PATCH 1/6] winegstreamer: Insert videoconvert into decoded-video streams.

[Zebediah Figura]

On 9/29/20 5:08 PM, Derek Lesho wrote:
> Signed-off-by: Derek Lesho <dlesho at codeweavers.com>
> ---
>  dlls/winegstreamer/media_source.c | 113 +++++++++++++++++++++++++-----
>  1 file changed, 97 insertions(+), 16 deletions(-)
> 
> diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
> index 5f3c43a0204..520f3c3177e 100644
> --- a/dlls/winegstreamer/media_source.c
> +++ b/dlls/winegstreamer/media_source.c
> @@ -384,6 +384,42 @@ static const IMFMediaStreamVtbl media_stream_vtbl =
>      media_stream_RequestSample
>  };
>  
> +/* Setup a chain of elements which should hopefully allow transformations to any IMFMediaType
> +   the user throws at us through gstreamer's caps negotiation. */
> +static HRESULT media_stream_resolve(struct media_stream *stream)

You return an HRESULT from this function, but don't use it anywhere.

This also strikes me as not a particularly great name; you aren't really
"resolving" anything here; rather, you're adding post-processing elements.

> +{
> +    GstCaps *source_caps = gst_pad_query_caps(stream->their_src, NULL);
> +
> +    if (!source_caps)
> +        return E_FAIL;
> +
> +    if (!strcmp(gst_structure_get_name(gst_caps_get_structure(source_caps, 0)), "video/x-raw"))
> +    {
> +        GstElement *videoconvert = gst_element_factory_make("videoconvert", NULL);
> +
> +        gst_bin_add(GST_BIN(stream->parent_source->container), videoconvert);
> +
> +        stream->my_sink = gst_element_get_static_pad(videoconvert, "sink");
> +
> +        assert(gst_pad_link(stream->their_src, stream->my_sink) == GST_PAD_LINK_OK);

This statement is done in both sides of the branch.

> +        assert(gst_element_link(videoconvert, stream->appsink));
> +
> +        gst_element_sync_state_with_parent(videoconvert);
> +
> +        g_object_set(stream->appsink, "caps", source_caps, NULL);

As is this one, but moreover, what's the point of doing this?

> +    }
> +    else
> +    {
> +        stream->my_sink = gst_element_get_static_pad(stream->appsink, "sink");
> +        g_object_set(stream->appsink, "caps", source_caps, NULL);
> +        assert(gst_pad_link(stream->their_src, stream->my_sink) == GST_PAD_LINK_OK);
> +    }
> +
> +    gst_caps_unref(source_caps);
> +
> +    return S_OK;
> +}
> +
>  static HRESULT new_media_stream(struct media_source *source, GstPad *pad, DWORD stream_id, struct media_stream **out_stream)
>  {
>      struct media_stream *object = heap_alloc_zero(sizeof(*object));
> @@ -414,8 +450,7 @@ static HRESULT new_media_stream(struct media_source *source, GstPad *pad, DWORD
>      g_object_set(object->appsink, "sync", FALSE, NULL);
>      g_object_set(object->appsink, "max-buffers", 5, NULL);
>  
> -    object->my_sink = gst_element_get_static_pad(object->appsink, "sink");
> -    gst_pad_link(object->their_src, object->my_sink);
> +    media_stream_resolve(object);
>  
>      gst_element_sync_state_with_parent(object->appsink);
>  
> @@ -435,28 +470,74 @@ static HRESULT media_stream_init_desc(struct media_stream *stream)
>  {
>      GstCaps *current_caps = gst_pad_get_current_caps(stream->their_src);
>      IMFMediaTypeHandler *type_handler;
> +    IMFMediaType **stream_types = NULL;
>      IMFMediaType *stream_type = NULL;
> +    DWORD type_count = 0;
> +    unsigned int i;
>      HRESULT hr;
> +    if (!strcmp(gst_structure_get_name(gst_caps_get_structure(current_caps, 0)), "video/x-raw"))
> +    {
> +        GstElementFactory *videoconvert_factory = gst_element_factory_find("videoconvert");
> +        /* output every format supported by videoconvert */
> +        const GList *template_list = gst_element_factory_get_static_pad_templates(videoconvert_factory);
> +        for (;template_list; template_list = template_list->next)
> +        {
> +            GstStaticPadTemplate *template = (GstStaticPadTemplate *)template_list->data;
> +            GstCaps *src_caps;
> +            GValueArray *formats;
> +            if (template->direction != GST_PAD_SRC)
> +                continue;
> +            src_caps = gst_static_pad_template_get_caps(template);
> +            gst_structure_get_list(gst_caps_get_structure(src_caps, 0), "format", &formats);
> +            stream_types = heap_alloc( sizeof(IMFMediaType*) * formats->n_values );
> +            for (i = 0; i < formats->n_values; i++)
> +            {
> +                GValue *format = g_value_array_get_nth(formats, i);
> +                GstCaps *modified_caps = gst_caps_copy(current_caps);
> +                gst_caps_set_value(modified_caps, "format", format);
> +                stream_types[type_count] = mf_media_type_from_caps(modified_caps);
> +                gst_caps_unref(modified_caps);
> +                if (stream_types[type_count])
> +                    type_count++;
> +            }
> +            g_value_array_free(formats);
> +            gst_caps_unref(src_caps);
> +            break;
> +        }

I think it would be far simpler to query the videoconvert element you've
actually just added.

> +    }
> +    else
> +    { I think in that case you would potentially call gst_caps_intersect
> +        stream_type = mf_media_type_from_caps(current_caps);
> +        if (stream_type)
> +        {
> +            stream_types = &stream_type;
> +            type_count = 1;
> +        }
> +    }
>  
> -    stream_type = mf_media_type_from_caps(current_caps);
>      gst_caps_unref(current_caps);
> -    if (!stream_type)
> +    if (!type_count)
> +    {
> +        ERR("Failed to establish an IMFMediaType from any of the possible stream caps!\n");
>          return E_FAIL;
> +    }
>  
> -    hr = MFCreateStreamDescriptor(stream->stream_id, 1, &stream_type, &stream->descriptor);
> -
> -    IMFMediaType_Release(stream_type);
> -
> -    if (FAILED(hr))
> -        return hr;
> +    if (FAILED(hr = MFCreateStreamDescriptor(stream->stream_id, type_count, stream_types, &stream->descriptor)))
> +        goto done;
>  
>      if (FAILED(hr = IMFStreamDescriptor_GetMediaTypeHandler(stream->descriptor, &type_handler)))
> -        return hr;
> -
> -    hr = IMFMediaTypeHandler_SetCurrentMediaType(type_handler, stream_type);
> -
> -    IMFMediaTypeHandler_Release(type_handler);
> -
> +        goto done;
> +
> +    if (FAILED(hr = IMFMediaTypeHandler_SetCurrentMediaType(type_handler, stream_types[0])))
> +        goto done;
> +
> +done:
> +    if (type_handler)
> +        IMFMediaTypeHandler_Release(type_handler);
> +    for (i = 0; i < type_count; i++)
> +        IMFMediaType_Release(stream_types[i]);
> +    if (stream_types != &stream_type)

You could probably avoid this sort of thing by just calling
MFCreateStreamDescriptor() inside of the if/else switch.

> +        heap_free(stream_types);
>      return hr;
>  }
>  
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20201002/89785649/attachment.sig>