[PATCH v4 3/3] winegstreamer: Implement IMFMediaStream::GetStreamDescriptor.

Zebediah Figura zfigura at codeweavers.com
Thu Sep 24 12:27:42 CDT 2020


On 9/21/20 2:01 PM, Derek Lesho wrote:
> Signed-off-by: Derek Lesho <dlesho at codeweavers.com>
> ---
> v3: Use pull-preroll and gst_pad_get_current_caps to establish media types, instead of a pad probe.
> ---
>  dlls/winegstreamer/gst_private.h  |   4 +
>  dlls/winegstreamer/media_source.c |  75 +++++++++++++-
>  dlls/winegstreamer/mfplat.c       | 166 ++++++++++++++++++++++++++++++
>  3 files changed, 240 insertions(+), 5 deletions(-)
> 
> diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h
> index ef07d3591e7..60b38a48f5a 100644
> --- a/dlls/winegstreamer/gst_private.h
> +++ b/dlls/winegstreamer/gst_private.h
> @@ -36,6 +36,7 @@
>  #include "winuser.h"
>  #include "dshow.h"
>  #include "strmif.h"
> +#include "mfobjects.h"
>  #include "wine/heap.h"
>  #include "wine/strmbase.h"
>  
> @@ -54,6 +55,9 @@ void start_dispatch_thread(void) DECLSPEC_HIDDEN;
>  
>  extern HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj) DECLSPEC_HIDDEN;
>  
> +HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN;
> +IMFMediaType *mf_media_type_from_caps(const GstCaps *caps) DECLSPEC_HIDDEN;
> +
>  HRESULT winegstreamer_stream_handler_create(REFIID riid, void **obj) DECLSPEC_HIDDEN;
>  
>  #endif /* __GST_PRIVATE_INCLUDED__ */
> diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
> index 462a646ee4f..a20b080d2a5 100644
> --- a/dlls/winegstreamer/media_source.c
> +++ b/dlls/winegstreamer/media_source.c
> @@ -48,13 +48,17 @@ struct media_stream
>      LONG ref;
>      struct media_source *parent_source;
>      IMFMediaEventQueue *event_queue;
> +    IMFStreamDescriptor *descriptor;
>      GstElement *appsink;
>      GstPad *their_src, *my_sink;
>      enum
>      {
>          STREAM_STUB,
> +        STREAM_INACTIVE,

What's the difference between these two states? There doesn't seem to be
a difference in this patch.

>          STREAM_SHUTDOWN,
>      } state;
> +    /* used when in STUB state: */
> +    DWORD stream_id;

What's the point of this comment? Is the stream ID not valid otherwise?

>  };
>  
>  struct media_source
> @@ -286,6 +290,8 @@ static ULONG WINAPI media_stream_Release(IMFMediaStream *iface)
>      {
>          if (stream->my_sink)
>              gst_object_unref(GST_OBJECT(stream->my_sink));
> +        if (stream->descriptor)
> +            IMFStreamDescriptor_Release(stream->descriptor);
>          if (stream->event_queue)
>              IMFMediaEventQueue_Release(stream->event_queue);
>          if (stream->parent_source)
> @@ -367,7 +373,10 @@ static HRESULT WINAPI media_stream_GetStreamDescriptor(IMFMediaStream* iface, IM
>      if (stream->state == STREAM_SHUTDOWN)
>          return MF_E_SHUTDOWN;
>  
> -    return E_NOTIMPL;
> +    IMFStreamDescriptor_AddRef(stream->descriptor);
> +    *descriptor = stream->descriptor;
> +
> +    return S_OK;
>  }
>  
>  static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown *token)
> @@ -397,7 +406,7 @@ static const IMFMediaStreamVtbl media_stream_vtbl =
>  };
>  
>  /* creates a stub stream */
> -static HRESULT new_media_stream(struct media_source *source, GstPad *pad, struct media_stream **out_stream)
> +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));
>      HRESULT hr;
> @@ -410,6 +419,7 @@ static HRESULT new_media_stream(struct media_source *source, GstPad *pad, struct
>      IMFMediaSource_AddRef(&source->IMFMediaSource_iface);
>      object->parent_source = source;
>      object->their_src = pad;
> +    object->stream_id = stream_id;
>  
>      object->state = STREAM_STUB;
>  
> @@ -427,8 +437,6 @@ static HRESULT new_media_stream(struct media_source *source, GstPad *pad, struct
>      g_object_set(object->appsink, "max-buffers", 5, NULL);
>  
>      object->my_sink = gst_element_get_static_pad(object->appsink, "sink");
> -    gst_pad_set_element_private(object->my_sink, object);
> -
>      gst_pad_link(object->their_src, object->my_sink);
>  
>      gst_element_sync_state_with_parent(object->appsink);
> @@ -445,6 +453,47 @@ fail:
>      return hr;
>  }
>  
> +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_type = NULL;
> +    HRESULT hr;
> +
> +    if (!current_caps)
> +    {
> +        hr = E_FAIL;
> +        goto fail;
> +    }

Can that happen?

Note that this "goto" and a couple others below could be replaced with
"return hr". In general I'm inclined to think goto isn't worthwhile when
there's only one line of cleanup.

(Note also that the return before the goto is a little redundant as is;
you release type_handler in both success and failure cases.)

> +
> +    stream_type = mf_media_type_from_caps(current_caps);
> +    gst_caps_unref(current_caps);
> +    if (!stream_type)
> +    {
> +        hr = E_FAIL;
> +        goto fail;
> +    }
> +
> +    if (FAILED(hr = MFCreateStreamDescriptor(stream->stream_id, 1, &stream_type, &stream->descriptor)))
> +        goto fail;
> +
> +    if (FAILED(hr = IMFStreamDescriptor_GetMediaTypeHandler(stream->descriptor, &type_handler)))
> +        goto fail;
> +
> +    if (FAILED(hr = IMFMediaTypeHandler_SetCurrentMediaType(type_handler, stream_type)))
> +        goto fail;
> +
> +    IMFMediaTypeHandler_Release(type_handler);
> +
> +    stream->state = STREAM_INACTIVE;
> +
> +    return S_OK;
> +    fail:

I find labels easier to read when not preceded by any spaces.

> +    if (type_handler)
> +        IMFMediaTypeHandler_Release(type_handler);
> +    return hr;
> +}
> +
>  static HRESULT WINAPI media_source_QueryInterface(IMFMediaSource *iface, REFIID riid, void **out)
>  {
>      struct media_source *source = impl_from_IMFMediaSource(iface);
> @@ -662,7 +711,7 @@ static void stream_added(GstElement *element, GstPad *pad, gpointer user)
>      if (gst_pad_get_direction(pad) != GST_PAD_SRC)
>          return;
>  
> -    if (FAILED(new_media_stream(source, pad, &stream)))
> +    if (FAILED(new_media_stream(source, pad, source->stream_count, &stream)))
>          return;
>  
>      if (!(new_stream_array = heap_realloc(source->streams, (source->stream_count + 1) * (sizeof(*new_stream_array)))))
> @@ -687,6 +736,8 @@ static void stream_removed(GstElement *element, GstPad *pad, gpointer user)
>          if (stream->their_src != pad)
>              continue;
>          stream->their_src = NULL;
> +        if (stream->state != STREAM_INACTIVE)
> +            stream->state = STREAM_INACTIVE;

This "if" is superfluous.

>      }
>  }
>  
> @@ -703,6 +754,7 @@ static HRESULT media_source_constructor(IMFByteStream *bytestream, struct media_
>          GST_STATIC_PAD_TEMPLATE("mf_src", GST_PAD_SRC, GST_PAD_ALWAYS, GST_STATIC_CAPS_ANY);
>  
>      struct media_source *object = heap_alloc_zero(sizeof(*object));
> +    unsigned int i;
>      HRESULT hr;
>      int ret;
>  
> @@ -772,6 +824,19 @@ static HRESULT media_source_constructor(IMFByteStream *bytestream, struct media_
>      }
>  
>      WaitForSingleObject(object->all_streams_event, INFINITE);
> +    for (i = 0; i < object->stream_count; i++)
> +    {
> +        GstSample *preroll;
> +        g_signal_emit_by_name(object->streams[i]->appsink, "pull-preroll", &preroll);

Calling gst_app_sink_pull_preroll() directly seems a little less
confusing (i.e. in the sense that I wouldn't have had to look up how
G_SIGNAL_ACTION works).

> +        hr = E_FAIL;
> +        if (!preroll || FAILED(hr = media_stream_init_desc(object->streams[i])))

I don't think you want the "!preroll" check; a stream might be empty.

> +        {
> +            ERR("Failed to finish initialization of media stream %p, hr %x.\n", object->streams[i], hr);
> +            IMFMediaStream_Release(&object->streams[i]->IMFMediaStream_iface);
> +            goto fail;
> +        }
> +        gst_sample_unref(preroll);
> +    }
>  
>      object->state = SOURCE_STOPPED;
>  
> diff --git a/dlls/winegstreamer/mfplat.c b/dlls/winegstreamer/mfplat.c
> index c996f06211e..2c2216f94b6 100644
> --- a/dlls/winegstreamer/mfplat.c
> +++ b/dlls/winegstreamer/mfplat.c
> @@ -16,6 +16,11 @@
>   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>   */
>  
> +#include "config.h"
> +#include <gst/gst.h>
> +
> +#include "gst_private.h"
> +
>  #include <stdarg.h>
>  
>  #include "gst_private.h"
> @@ -436,3 +441,164 @@ HRESULT mfplat_get_class_object(REFCLSID rclsid, REFIID riid, void **obj)
>  
>      return CLASS_E_CLASSNOTAVAILABLE;
>  }
> +
> +static const struct
> +{
> +    const GUID *subtype;
> +    GstVideoFormat format;
> +}
> +uncompressed_video_formats[] =
> +{
> +    {&MFVideoFormat_ARGB32,  GST_VIDEO_FORMAT_BGRA},
> +    {&MFVideoFormat_RGB32,   GST_VIDEO_FORMAT_BGRx},
> +    {&MFVideoFormat_RGB24,   GST_VIDEO_FORMAT_BGR},
> +    {&MFVideoFormat_RGB565,  GST_VIDEO_FORMAT_BGR16},
> +    {&MFVideoFormat_RGB555,  GST_VIDEO_FORMAT_BGR15},
> +};
> +
> +/* returns NULL if doesn't match exactly */
> +IMFMediaType *mf_media_type_from_caps(const GstCaps *caps)
> +{
> +    IMFMediaType *media_type;
> +    GstStructure *info;
> +    const char *mime_type;
> +
> +    if (TRACE_ON(mfplat))
> +    {
> +        gchar *human_readable = gst_caps_to_string(caps);
> +        TRACE("caps = %s\n", debugstr_a(human_readable));
> +        g_free(human_readable);
> +    }
> +
> +    if (FAILED(MFCreateMediaType(&media_type)))
> +        return NULL;
> +
> +    info = gst_caps_get_structure(caps, 0);
> +    mime_type = gst_structure_get_name(info);
> +
> +    if (!strncmp(mime_type, "video", 5))
> +    {
> +        GstVideoInfo video_info;
> +
> +        if (!gst_video_info_from_caps(&video_info, caps))
> +        {
> +            return NULL;
> +        }
> +
> +        IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Video);
> +
> +        IMFMediaType_SetUINT64(media_type, &MF_MT_FRAME_SIZE, ((UINT64)video_info.width << 32) | video_info.height);
> +
> +        IMFMediaType_SetUINT64(media_type, &MF_MT_FRAME_RATE, ((UINT64)video_info.fps_n << 32) | video_info.fps_d);
> +
> +        if (!strcmp(mime_type, "video/x-raw"))
> +        {
> +            GUID fourcc_subtype = MFVideoFormat_Base;
> +            unsigned int i;
> +
> +            IMFMediaType_SetUINT32(media_type, &MF_MT_COMPRESSED, FALSE);
> +
> +            /* First try FOURCC */
> +            if ((fourcc_subtype.Data1 = gst_video_format_to_fourcc(video_info.finfo->format)))
> +            {
> +                IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &fourcc_subtype);
> +            }
> +            else
> +            {
> +                for (i = 0; i < ARRAY_SIZE(uncompressed_video_formats); i++)
> +                {
> +                    if (uncompressed_video_formats[i].format == video_info.finfo->format)
> +                    {
> +                        IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, uncompressed_video_formats[i].subtype);
> +                        break;
> +                    }
> +                }
> +                if (i == ARRAY_SIZE(uncompressed_video_formats))
> +                {
> +                    FIXME("Unrecognized uncompressed video format %x\n", video_info.finfo->format);

"%#x" is usually a good idea; you could also use
gst_video_format_to_string().

> +                    IMFMediaType_Release(media_type);
> +                    return NULL;
> +                }
> +            }
> +        }
> +        else
> +        {
> +            FIXME("Unrecognized video format %s\n", mime_type);
> +            return NULL;
> +        }
> +    }
> +    else if (!strncmp(mime_type, "audio", 5))
> +    {
> +        gint rate, channels, bitrate;
> +        guint64 channel_mask;
> +        IMFMediaType_SetGUID(media_type, &MF_MT_MAJOR_TYPE, &MFMediaType_Audio);
> +
> +        if (gst_structure_get_int(info, "rate", &rate))
> +            IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_SAMPLES_PER_SECOND, rate);
> +
> +        if (gst_structure_get_int(info, "channels", &channels))
> +            IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_NUM_CHANNELS, channels);
> +
> +        if (gst_structure_get(info, "channel-mask", GST_TYPE_BITMASK, &channel_mask, NULL))
> +            IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_CHANNEL_MASK, channel_mask);
> +
> +        if (gst_structure_get_int(info, "bitrate", &bitrate))
> +            IMFMediaType_SetUINT32(media_type, &MF_MT_AVG_BITRATE, bitrate);
> +
> +        if (!strcmp(mime_type, "audio/x-raw"))
> +        {
> +            GstAudioInfo audio_info;
> +
> +            if (gst_audio_info_from_caps(&audio_info, caps))

Early return could save a level of indentation here.

> +            {
> +                DWORD depth = GST_AUDIO_INFO_DEPTH(&audio_info);
> +
> +                /* validation */
> +                if ((audio_info.finfo->flags & GST_AUDIO_FORMAT_FLAG_INTEGER && depth > 8) ||
> +                    (audio_info.finfo->flags & GST_AUDIO_FORMAT_FLAG_SIGNED && depth <= 8) ||
> +                    (audio_info.finfo->endianness != G_LITTLE_ENDIAN && depth > 8))
> +                {
> +                    IMFMediaType_Release(media_type);
> +                    return NULL;
> +                }
> +
> +                /* conversion */
> +                switch (audio_info.finfo->flags)
> +                {
> +                    case GST_AUDIO_FORMAT_FLAG_FLOAT:
> +                        IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_Float);
> +                        break;
> +                    case GST_AUDIO_FORMAT_FLAG_INTEGER:
> +                    case GST_AUDIO_FORMAT_FLAG_SIGNED:
> +                        IMFMediaType_SetGUID(media_type, &MF_MT_SUBTYPE, &MFAudioFormat_PCM);
> +                        break;
> +                    default:
> +                        FIXME("Unrecognized audio format %x\n", audio_info.finfo->format);
> +                        IMFMediaType_Release(media_type);
> +                        return NULL;
> +                }
> +
> +                IMFMediaType_SetUINT32(media_type, &MF_MT_AUDIO_BITS_PER_SAMPLE, depth);
> +            }
> +            else
> +            {
> +                ERR("Failed to get caps audio info\n");
> +                IMFMediaType_Release(media_type);
> +                return NULL;
> +            }
> +        }
> +        else
> +        {
> +            FIXME("Unrecognized audio format %s\n", mime_type);
> +            IMFMediaType_Release(media_type);
> +            return NULL;
> +        }
> +    }
> +    else
> +    {
> +        IMFMediaType_Release(media_type);
> +        return NULL;
> +    }
> +
> +    return media_type;
> +}
> 

-------------- 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/20200924/f5bb77a3/attachment-0001.sig>


More information about the wine-devel mailing list