On 3/26/20 5:13 PM, Derek Lesho wrote:
On 3/26/20 12:23 AM, Nikolay Sivov wrote:
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.
What do you mean? What might not be correct?
Failing to return descriptor.
+ 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.
Right now I haven't implemented pausing/seeking, shouldn't we wait until we support it before adding the characteristics?
Sure, I don't know why I assumed seeking was supposed to work.
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.