[PATCH 08/16] winegstreamer: Implement media source on gstreamer.

[Nikolay Sivov]

On 3/26/20 3:12 AM, Derek Lesho wrote:
> +static ULONG WINAPI media_stream_Release(IMFMediaStream *iface)
> +{
> +    struct media_stream *This = impl_from_IMFMediaStream(iface);
> +
> +    ULONG ref = InterlockedDecrement(&This->ref);
> +
> +    TRACE("(%p) ref=%u\n", This, ref);
> +
> +    if (!ref)
> +    {
> +        if (This->state != STREAM_SHUTDOWN)
> +            ERR("incomplete cleanup\n");
> +        heap_free(This);
> +    }
> +
> +    return ref;
> +}
Error message here is redundant I think. The point of shutdown is to 
break circular references,
where source holds streams references and every stream hold source 
reference. So in theory
you shouldn't be able to release unless you shut it down.

> +static HRESULT WINAPI media_stream_GetEvent(IMFMediaStream *iface, DWORD flags, IMFMediaEvent **event)
> +{
> +    struct media_stream *This = impl_from_IMFMediaStream(iface);
> +
> +    TRACE("(%p)->(%#x, %p)\n", This, flags, event);
> +
> +    if (This->state == STREAM_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    return IMFMediaEventQueue_GetEvent(This->event_queue, flags, event);
> +}
Same as with the source, you only need to shutdown the queue and it will 
return MF_E_SHUTDOWN for you.

> +static HRESULT WINAPI media_stream_GetStreamDescriptor(IMFMediaStream* iface, IMFStreamDescriptor **descriptor)
> +{
> +    struct media_stream *This = impl_from_IMFMediaStream(iface);
> +
> +    TRACE("(%p)->(%p)\n", This, descriptor);
> +
> +    if (This->state == STREAM_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    IMFStreamDescriptor_AddRef(This->descriptor);
> +    *descriptor = This->descriptor;
> +
> +    return S_OK;
> +}
It's not obvious that it's correct but could be problematic to test 
properly, because it could differ between implementations.

> +    req = heap_alloc(sizeof(*req));
> +    if (token)
> +        IUnknown_AddRef(token);
> +    req->token = token;
> +    list_add_tail(&This->sample_requests, &req->entry);
This should be protected, there is no guarantee client is using the same 
thread to make requests, or serializes them.

> @@ -118,28 +684,102 @@ static HRESULT WINAPI media_source_GetCharacteristics(IMFMediaSource *iface, DWO
>   {
>       struct media_source *This = impl_from_IMFMediaSource(iface);
>   
> -    FIXME("(%p)->(%p): stub\n", This, characteristics);
> +    TRACE("(%p)->(%p)\n", This, characteristics);
>   
> -    return E_NOTIMPL;
> +    if (This->state == SOURCE_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    *characteristics = 0;
> +
> +    return S_OK;
>   }
Returning 0 flags will prevent seeking. It should be derived from 
bytestream caps + gstreamer object constraints if any. There is a number 
of flags,
but CAN_PAUSE/CAN_SEEK are probably most important ones.

>   static HRESULT WINAPI media_source_CreatePresentationDescriptor(IMFMediaSource *iface, IMFPresentationDescriptor **descriptor)
>   {
>       struct media_source *This = impl_from_IMFMediaSource(iface);
>   
> -    FIXME("(%p)->(%p): stub\n", This, descriptor);
> +    TRACE("(%p)->(%p)\n", This, descriptor);
>   
> -    return E_NOTIMPL;
> +    if (This->state == SOURCE_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    if (!(This->pres_desc))
> +    {
> +        return MF_E_NOT_INITIALIZED;
> +    }
> +
> +    IMFPresentationDescriptor_Clone(This->pres_desc, descriptor);
> +
> +    return S_OK;
>   }
Is NOT_INITIALIZED path reachable? I think it's generally assumed that 
once you got a source you can get its descriptor, even if stream 
configuration
can change dynamically later.