Merge branch 'media_source_ref' into 'master'

winegstreamer: Fix memory/resource leaks in media sources.

See merge request wine/wine!6783

dlls/mfplat/tests/mfplat.c

@@ -986,12 +986,14 @@ static BOOL get_event(IMFMediaEventGenerator *generator, MediaEventType expected
                 ok(hr == S_OK, "Failed to get value of event, hr %#lx.\n", hr);
             }
 
+            IMFMediaEvent_Release(callback->media_event);
             break;
         }
+
+        if (callback->media_event)
+            IMFMediaEvent_Release(callback->media_event);
     }
 
-    if (callback->media_event)
-        IMFMediaEvent_Release(callback->media_event);
     IMFAsyncCallback_Release(&callback->IMFAsyncCallback_iface);
 
     return ret;
@@ -1319,8 +1321,10 @@ static void test_source_resolver(void)
     ok(mediasource != NULL, "got %p\n", mediasource);
     ok(obj_type == MF_OBJECT_MEDIASOURCE, "got %d\n", obj_type);
 
+    /* Test that no extra refs to mediasource are held if Start() was not called */
+    EXPECT_REF(mediasource, 1);
+
     refcount = IMFMediaSource_Release(mediasource);
-    todo_wine
     ok(!refcount, "Unexpected refcount %ld\n", refcount);
     IMFByteStream_Release(stream);
 
@@ -1407,15 +1411,21 @@ static void test_source_resolver(void)
     ok(mediasource != NULL, "got %p\n", mediasource);
     ok(obj_type == MF_OBJECT_MEDIASOURCE, "got %d\n", obj_type);
 
+    EXPECT_REF(mediasource, 1);
+
     check_interface(mediasource, &IID_IMFGetService, TRUE);
     check_service_interface(mediasource, &MF_RATE_CONTROL_SERVICE, &IID_IMFRateSupport, TRUE);
 
     hr = IMFMediaSource_QueryInterface(mediasource, &IID_IMFGetService, (void**)&get_service);
     ok(hr == S_OK, "Failed to get service interface, hr %#lx.\n", hr);
 
+    EXPECT_REF(mediasource, 2);
+
     hr = IMFGetService_GetService(get_service, &MF_RATE_CONTROL_SERVICE, &IID_IMFRateSupport, (void**)&rate_support);
     ok(hr == S_OK, "Failed to get rate support interface, hr %#lx.\n", hr);
 
+    EXPECT_REF(mediasource, 3);
+
     hr = IMFRateSupport_GetFastestRate(rate_support, MFRATE_FORWARD, FALSE, &rate);
     ok(hr == S_OK, "Failed to query fastest rate, hr %#lx.\n", hr);
     ok(rate == 1e6f, "Unexpected fastest rate %f.\n", rate);
@@ -1504,6 +1514,11 @@ static void test_source_resolver(void)
     hr = IMFMediaSource_Start(mediasource, descriptor, &GUID_NULL, &var);
     ok(hr == S_OK, "Failed to start media source, hr %#lx.\n", hr);
 
+    /* The stream holds a reference. It is unclear which object holds the fifth
+     * reference in Windows, but it's released after MENewStream is retrieved. */
+    todo_wine
+    EXPECT_REF(mediasource, 5);
+
     video_stream = NULL;
     if (get_event((IMFMediaEventGenerator *)mediasource, MENewStream, &var))
     {
@@ -1511,6 +1526,8 @@ static void test_source_resolver(void)
         video_stream = (IMFMediaStream *)var.punkVal;
     }
 
+    EXPECT_REF(mediasource, 4);
+
     hr = IMFMediaSource_Pause(mediasource);
     ok(hr == S_OK, "Failed to pause media source, hr %#lx.\n", hr);
     if (get_event((IMFMediaEventGenerator *)mediasource, MESourcePaused, &var))
@@ -1610,7 +1627,6 @@ static void test_source_resolver(void)
 
     get_event((IMFMediaEventGenerator *)mediasource, MEEndOfPresentation, NULL);
 
-    IMFMediaStream_Release(video_stream);
     IMFMediaTypeHandler_Release(handler);
     IMFPresentationDescriptor_Release(descriptor);
 
@@ -1619,14 +1635,24 @@ static void test_source_resolver(void)
     hr = IMFMediaSource_Shutdown(mediasource);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
 
+    /* During shutdown, circular references such as source <-> stream should be released. */
+    EXPECT_REF(mediasource, 3);
+
     ok(bytestream_closed, "Missing IMFByteStream::Close call\n");
 
     hr = IMFMediaSource_CreatePresentationDescriptor(mediasource, NULL);
     ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr);
 
     IMFRateSupport_Release(rate_support);
+
+    EXPECT_REF(mediasource, 2);
+
     IMFGetService_Release(get_service);
-    IMFMediaSource_Release(mediasource);
+
+    /* Holding a reference to the video stream does not prevent release of the media source. */
+    refcount = IMFMediaSource_Release(mediasource);
+    ok(!refcount, "Unexpected refcount %ld\n", refcount);
+
     IMFByteStream_Release(stream);
 
     /* Create directly through scheme handler. */
@@ -1659,6 +1685,12 @@ static void test_source_resolver(void)
 
     IMFSourceResolver_Release(resolver);
 
+    hr = IMFMediaStream_GetMediaSource(video_stream, &mediasource);
+    ok(hr == MF_E_SHUTDOWN, "Unexpected hr %#lx.\n", hr);
+
+    refcount = IMFMediaStream_Release(video_stream);
+    ok(!refcount, "Unexpected refcount %ld\n", refcount);
+
     hr = MFShutdown();
     ok(hr == S_OK, "Failed to shut down, hr %#lx.\n", hr);
 

dlls/winegstreamer/media_source.c

@@ -187,6 +187,7 @@ struct media_source
     UINT64 duration;
 
     IMFStreamDescriptor **descriptors;
+    wg_parser_stream_t *wg_streams;
     struct media_stream **streams;
     ULONG stream_count;
 
@@ -542,6 +543,8 @@ static void flush_token_queue(struct media_stream *stream, BOOL send)
             IUnknown *op;
             HRESULT hr;
 
+            assert(stream->media_source);
+
             if (SUCCEEDED(hr = source_create_async_op(SOURCE_ASYNC_REQUEST_SAMPLE, &op)))
             {
                 struct source_async_command *command = impl_from_async_command_IUnknown(op);
@@ -582,6 +585,9 @@ static HRESULT media_stream_start(struct media_stream *stream, BOOL active, BOOL
             &GUID_NULL, S_OK, position);
 }
 
+static HRESULT media_stream_create(IMFMediaSource *source, IMFStreamDescriptor *descriptor,
+        wg_parser_stream_t wg_stream, struct media_stream **out);
+
 static HRESULT media_source_start(struct media_source *source, IMFPresentationDescriptor *descriptor,
         GUID *format, PROPVARIANT *position)
 {
@@ -596,6 +602,26 @@ static HRESULT media_source_start(struct media_source *source, IMFPresentationDe
     if (source->state == SOURCE_SHUTDOWN)
         return MF_E_SHUTDOWN;
 
+    /* if starting for the first time, create the streams */
+    if (source->stream_count && !source->streams[0])
+    {
+        assert(source->state == SOURCE_STOPPED);
+
+        for (i = 0; i < source->stream_count; ++i)
+        {
+            wg_parser_stream_t wg_stream = source->wg_streams[i];
+            struct media_stream *stream;
+
+            if (FAILED(hr = media_stream_create(&source->IMFMediaSource_iface,
+                    source->descriptors[i], wg_stream, &stream)))
+                return hr;
+
+            source->streams[i] = stream;
+        }
+        free(source->wg_streams);
+        source->wg_streams = NULL;
+    }
+
     /* seek to beginning on stop->play */
     if (source->state == SOURCE_STOPPED && position->vt == VT_EMPTY)
     {
@@ -962,7 +988,11 @@ static ULONG WINAPI media_stream_Release(IMFMediaStream *iface)
 
     if (!ref)
     {
-        IMFMediaSource_Release(stream->media_source);
+        if (stream->media_source)
+        {
+            IMFMediaSource_Release(stream->media_source);
+            stream->media_source = NULL;
+        }
         IMFStreamDescriptor_Release(stream->descriptor);
         IMFMediaEventQueue_Release(stream->event_queue);
         flush_token_queue(stream, FALSE);
@@ -1012,13 +1042,24 @@ static HRESULT WINAPI media_stream_QueueEvent(IMFMediaStream *iface, MediaEventT
 static HRESULT WINAPI media_stream_GetMediaSource(IMFMediaStream *iface, IMFMediaSource **out)
 {
     struct media_stream *stream = impl_from_IMFMediaStream(iface);
-    struct media_source *source = impl_from_IMFMediaSource(stream->media_source);
+    IMFMediaSource *source_iface = stream->media_source;
+    struct media_source *source;
     HRESULT hr = S_OK;
 
     TRACE("%p, %p.\n", iface, out);
 
+    if (!source_iface)
+        return MF_E_SHUTDOWN;
+
+    source = impl_from_IMFMediaSource(source_iface);
+
     EnterCriticalSection(&source->cs);
 
+    /* A shutdown state can occur here if shutdown was in progress in another
+     * thread when we got the source pointer above. The source object must
+     * still exist and we cannot reasonably handle a case where the source has
+     * been destroyed at this point in a get/request method without introducing
+     * a critical section into the stream object. */
     if (source->state == SOURCE_SHUTDOWN)
         hr = MF_E_SHUTDOWN;
     else
@@ -1035,11 +1076,17 @@ static HRESULT WINAPI media_stream_GetMediaSource(IMFMediaStream *iface, IMFMedi
 static HRESULT WINAPI media_stream_GetStreamDescriptor(IMFMediaStream* iface, IMFStreamDescriptor **descriptor)
 {
     struct media_stream *stream = impl_from_IMFMediaStream(iface);
-    struct media_source *source = impl_from_IMFMediaSource(stream->media_source);
+    IMFMediaSource *source_iface = stream->media_source;
+    struct media_source *source;
     HRESULT hr = S_OK;
 
     TRACE("%p, %p.\n", iface, descriptor);
 
+    if (!source_iface)
+        return MF_E_SHUTDOWN;
+
+    source = impl_from_IMFMediaSource(source_iface);
+
     EnterCriticalSection(&source->cs);
 
     if (source->state == SOURCE_SHUTDOWN)
@@ -1058,12 +1105,18 @@ static HRESULT WINAPI media_stream_GetStreamDescriptor(IMFMediaStream* iface, IM
 static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown *token)
 {
     struct media_stream *stream = impl_from_IMFMediaStream(iface);
-    struct media_source *source = impl_from_IMFMediaSource(stream->media_source);
+    IMFMediaSource *source_iface = stream->media_source;
+    struct media_source *source;
     IUnknown *op;
     HRESULT hr;
 
     TRACE("%p, %p.\n", iface, token);
 
+    if (!source_iface)
+        return MF_E_SHUTDOWN;
+
+    source = impl_from_IMFMediaSource(source_iface);
+
     EnterCriticalSection(&source->cs);
 
     if (source->state == SOURCE_SHUTDOWN)
@@ -1569,10 +1622,18 @@ static HRESULT WINAPI media_source_Shutdown(IMFMediaSource *iface)
     {
         struct media_stream *stream = source->streams[source->stream_count];
         IMFStreamDescriptor_Release(source->descriptors[source->stream_count]);
-        IMFMediaEventQueue_Shutdown(stream->event_queue);
+        if (stream)
-        IMFMediaStream_Release(&stream->IMFMediaStream_iface);
+        {
+            IMFMediaEventQueue_Shutdown(stream->event_queue);
+            /* Media Foundation documentation says circular references such as
+             * those between the source and its streams should be released here. */
+            IMFMediaSource_Release(stream->media_source);
+            stream->media_source = NULL;
+            IMFMediaStream_Release(&stream->IMFMediaStream_iface);
+        }
     }
     free(source->descriptors);
+    free(source->wg_streams);
     free(source->streams);
 
     LeaveCriticalSection(&source->cs);
@@ -1606,13 +1667,12 @@ static void media_source_init_descriptors(struct media_source *source)
 
     for (i = 0; i < source->stream_count; i++)
     {
-        struct media_stream *stream = source->streams[i];
+        IMFStreamDescriptor *descriptor = source->descriptors[i];
-        IMFStreamDescriptor *descriptor = stream->descriptor;
 
-        if (FAILED(hr = stream_descriptor_set_tag(descriptor, stream->wg_stream,
+        if (FAILED(hr = stream_descriptor_set_tag(descriptor, source->wg_streams[i],
                 &MF_SD_LANGUAGE, WG_PARSER_TAG_LANGUAGE)))
             WARN("Failed to set stream descriptor language, hr %#lx\n", hr);
-        if (FAILED(hr = stream_descriptor_set_tag(descriptor, stream->wg_stream,
+        if (FAILED(hr = stream_descriptor_set_tag(descriptor, source->wg_streams[i],
                 &MF_SD_STREAM_NAME, WG_PARSER_TAG_NAME)))
             WARN("Failed to set stream descriptor name, hr %#lx\n", hr);
     }
@@ -1665,6 +1725,7 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc
     stream_count = wg_parser_get_stream_count(parser);
 
     if (!(object->descriptors = calloc(stream_count, sizeof(*object->descriptors)))
+            || !(object->wg_streams = calloc(stream_count, sizeof(*object->wg_streams)))
             || !(object->streams = calloc(stream_count, sizeof(*object->streams))))
     {
         hr = E_OUTOFMEMORY;
@@ -1673,24 +1734,23 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc
 
     for (i = 0; i < stream_count; ++i)
     {
+        /* It is valid to create and release a MF source without ever calling Start() and
+         * Shutdown(). Each MF stream holds a reference to the source, and that ref should
+         * be released in Shutdown(), so streams are not created here.
+         * The wg streams are needed now to get the format and duration. Their buffer is
+         * freed in Start(). */
         wg_parser_stream_t wg_stream = wg_parser_get_stream(object->wg_parser, i);
         IMFStreamDescriptor *descriptor;
-        struct media_stream *stream;
         struct wg_format format;
 
         wg_parser_stream_get_current_format(wg_stream, &format);
         if (FAILED(hr = stream_descriptor_create(i, &format, &descriptor)))
             goto fail;
-        if (FAILED(hr = media_stream_create(&object->IMFMediaSource_iface, descriptor, wg_stream, &stream)))
-        {
-            IMFStreamDescriptor_Release(descriptor);
-            goto fail;
-        }
 
         object->duration = max(object->duration, wg_parser_stream_get_duration(wg_stream));
         IMFStreamDescriptor_AddRef(descriptor);
         object->descriptors[i] = descriptor;
-        object->streams[i] = stream;
+        object->wg_streams[i] = wg_stream;
         object->stream_count++;
     }
 
@@ -1704,13 +1764,12 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc
 fail:
     WARN("Failed to construct MFMediaSource, hr %#lx.\n", hr);
 
-    while (object->streams && object->stream_count--)
+    while (object->descriptors && object->stream_count--)
     {
-        struct media_stream *stream = object->streams[object->stream_count];
         IMFStreamDescriptor_Release(object->descriptors[object->stream_count]);
-        IMFMediaStream_Release(&stream->IMFMediaStream_iface);
     }
     free(object->descriptors);
+    free(object->wg_streams);
     free(object->streams);
 
     if (stream_count != UINT_MAX)