Re: [PATCH 1/3] winegstreamer: Duplicate source shutdown path into constructor with leak fixes.

[Zebediah Figura]

On 9/10/21 12:04 PM, Derek Lesho wrote:
> @@ -857,6 +854,8 @@ done:
>           IMFMediaTypeHandler_Release(type_handler);
>       for (i = 0; i < type_count; i++)
>           IMFMediaType_Release(stream_types[i]);
> - if (FAILED(hr))
> -
>    IMFStreamDescriptor_Release(stream->descriptor);
> return hr;
> }

There are failure paths from media_stream_init_desc() with FAILED(hr) && 
!stream->descriptor.

...
> @@ -1392,8 +1385,40 @@ static HRESULT media_source_constructor(IMFByteStream *bytestream, struct media_
>       fail:
>       WARN("Failed to construct MFMediaSource, hr %#x.\n", hr);
> - free(descriptors);
> - IMFMediaSource_Release(&object->IMFMediaSource_iface);
> - if (descriptors)
> - {
> -
>    for(i = 0; i < object->stream_count; i++)
> -
>        IMFStreamDescriptor_Release(descriptors[i]);
> -
>    free(descriptors);
> - }
> - for (i = 0; i < object->stream_count; i++)
> - {
> -
>    struct media_stream *stream = object->streams[i];
> -
> -
>    IMFMediaEventQueue_Release(stream->event_queue);
> -
>    IMFStreamDescriptor_Release(stream->descriptor);
> -
>    IMFMediaSource_Release(&stream->parent_source->IMFMediaSource_iface);
> -
> -
>    free(stream);
> - }
> - if (object->streams)
> -
>    free(object->streams);

As long as we're here, this if() is unnecessary.
> - if (stream_count)
> -
>    unix_funcs->wg_parser_disconnect(object->wg_parser);

This doesn't work; the parser could have zero streams. Granted, that 
usually means an error condition on the GStreamer side, but it's still 
worth handling right.
> - if (object->read_thread)
> - {
> -
>    object->read_thread_shutdown = true;
> -
>    WaitForSingleObject(object->read_thread, INFINITE);
> -
>    CloseHandle(object->read_thread);
> - }
> - if (object->wg_parser)
> -
>    unix_funcs->wg_parser_destroy(object->wg_parser);
> - if (object->async_commands_queue)
> -
>    MFUnlockWorkQueue(object->async_commands_queue);
> - if (object->event_queue)
> -
>    IMFMediaEventQueue_Release(object->event_queue);
> - IMFByteStream_Release(object->byte_stream);
> - free(object);
>  return hr;
> }