[PATCH v2 08/11] winegstreamer: Implement generic media source.

[Nikolay Sivov]

On 2/11/20 1:32 AM, Derek Lesho wrote:
> +static HRESULT WINAPI media_stream_QueueEvent(IMFMediaStream *iface, MediaEventType event_type, REFGUID ext_type,
> +        HRESULT hr, const PROPVARIANT *value)
> +{
> +    struct media_stream *This = impl_from_IMFMediaStream(iface);
> +
> +    TRACE("(%p)->(%d, %s, %#x, %p)\n", This, event_type, debugstr_guid(ext_type), hr, value);
> +
> +    if (This->state == STREAM_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    return IMFMediaEventQueue_QueueEventParamVar(This->event_queue, event_type, ext_type, hr, value);
> +}
Same pattern as with sources, this should simply forward to event queue 
method.

> +static HRESULT WINAPI media_stream_RequestSample(IMFMediaStream *iface, IUnknown *token)
> +{
> +    struct media_stream *This = impl_from_IMFMediaStream(iface);
> +    struct sample_request *req;
> +
> +    TRACE("(%p)->(%p)\n", iface, token);
> +
> +    if (This->state == STREAM_SHUTDOWN)
> +        return MF_E_SHUTDOWN;
> +
> +    if (This->state == STREAM_INACTIVE || This->state == STREAM_ENABLED)
> +    {
> +        WARN("Stream isn't active\n");
> +        return MF_E_MEDIA_SOURCE_WRONGSTATE;
> +    }
> +
> +    if (This->eos && !This->pending_samples)
> +        return MF_E_END_OF_STREAM;
> +
> +    req = heap_alloc(sizeof(*req));
> +    if (token)
> +        IUnknown_AddRef(token);
> +    req->token = token;
> +    list_add_tail(&This->sample_requests, &req->entry);
> +
> +    stream_dispatch_samples(This);
> +
> +    return S_OK;
> +}
This should be protected with critical section as well.

> +static GstFlowReturn stream_new_sample(GstElement *appsink, gpointer user)
> +{
> +    struct media_stream *This = (struct media_stream *) user;
> +
> +    TRACE("(%p) got sample\n", This);
> +
> +    if (This->state == STREAM_INACTIVE)
> +    {
> +        ERR("got sample on inactive stream\n");
> +    }
> +
> +    This->pending_samples++;
> +    stream_dispatch_samples(This);
> +    return GST_FLOW_OK;
> +}
Does it matter how many samples sink had accumulated so far? As I 
understand it's meant to hold fixed number of them and ask to decode 
more when you start pull them out. So basically when iterating through 
requests list, sink will simply fail to return any new samples, and 
that's how you could stop iterating it.

> +        /* Get the sample from the appsink, then construct an IMFSample */
> +        /* We do this in the dispatch function so we can have appsink buffer for us */
> +        {
> +            GstSample *gst_sample;
> +
> +            TRACE("Trying to pull sample\n");
> +
> +            g_signal_emit_by_name (This->appsink, "pull-sample", &gst_sample);
> +            if (!gst_sample)
> +            {
> +                ERR("Appsink has no samples and pending_samples != 0\n");
> +                break;
> +            }
> +
> +            sample = mf_sample_from_gst_sample(gst_sample);
> +        }
> +
> +        if (req->token)
> +        {
> +            IMFSample_SetUnknown(sample, &MFSampleExtension_Token, req->token);
> +        }
> +
> +        IMFMediaEventQueue_QueueEventParamUnk(This->event_queue, MEMediaSample, &GUID_NULL, S_OK, (IUnknown *)sample);
> +
> +        if (req->token)
> +        {
> +            IUnknown_Release(req->token);
> +        }
> +
> +        list_remove(&req->entry);
> +
> +        This->pending_samples--;
It looks like this leaks samples, event will hold a reference. It also 
leaks 'req' structure.

> +IMFSample* mf_sample_from_gst_sample(GstSample *in)
> +{
> +    GstBuffer *gst_buffer;
> +    gsize buf_size;
> +    BYTE *buf_data;
> +    LONGLONG duration, time;
> +    IMFMediaBuffer *mf_buffer;
> +    IMFSample *out = NULL;
> +
> +    gst_buffer = gst_sample_get_buffer(in);
> +
> +    buf_size = gst_buffer_get_size(gst_buffer);
> +
> +    duration = GST_BUFFER_DURATION(gst_buffer);
> +    time = GST_BUFFER_DTS(gst_buffer);
> +
> +    MFCreateMemoryBuffer(buf_size, &mf_buffer);
> +    IMFMediaBuffer_Lock(mf_buffer, &buf_data, NULL, NULL);
> +    gst_buffer_extract(gst_buffer, 0, buf_data, buf_size);
> +    IMFMediaBuffer_Unlock(mf_buffer);
> +
> +    MFCreateSample(&out);
> +    IMFSample_AddBuffer(out, mf_buffer);
> +    IMFMediaBuffer_Release(mf_buffer);
> +    IMFSample_SetSampleDuration(out, duration / 100);
> +    IMFSample_SetSampleTime(out, time / 100);
> +
> +    return out;
> +}
I think we should consider having a wrapper reusing gstreamer memory, so 
we don't have to copy.