[PATCH 3/5] amstream: Implement MediaStreamFilter::SupportSeeking.

[Anton Baskanov]

On Tuesday, 2 June 2020 23:04:00 +07 you wrote:
> On 6/2/20 10:54 AM, Zebediah Figura wrote:
> > On 6/1/20 11:13 PM, Anton Baskanov wrote:
> >> Signed-off-by: Anton Baskanov <baskanov at gmail.com>
> >> ---
> >> 
> >>  dlls/amstream/filter.c         | 249 ++++++++++++++++++++++++-
> >>  dlls/amstream/tests/amstream.c | 323 +++++++++++++++++++++++++++++++++
> >>  2 files changed, 568 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/dlls/amstream/filter.c b/dlls/amstream/filter.c
> >> index 4b5e184f04..643cfcdbe4 100644
> >> --- a/dlls/amstream/filter.c
> >> +++ b/dlls/amstream/filter.c
> >> @@ -163,6 +163,7 @@ static const IEnumPinsVtbl enum_pins_vtbl =
> >> 
> >>  struct filter
> >>  {
> >>  
> >>      IMediaStreamFilter IMediaStreamFilter_iface;
> >> 
> >> +    IMediaSeeking IMediaSeeking_iface;
> >> 
> >>      LONG refcount;
> >>      CRITICAL_SECTION cs;
> >> 
> >> @@ -171,6 +172,7 @@ struct filter
> >> 
> >>      IFilterGraph *graph;
> >>      ULONG nb_streams;
> >>      IAMMediaStream **streams;
> >> 
> >> +    IAMMediaStream *seekable_stream;
> >> 
> >>      FILTER_STATE state;
> >>  
> >>  };
> >> 
> >> @@ -181,6 +183,8 @@ static inline struct filter
> >> *impl_from_IMediaStreamFilter(IMediaStreamFilter *if>> 
> >>  static HRESULT WINAPI filter_QueryInterface(IMediaStreamFilter *iface,
> >>  REFIID riid, void **ret_iface) {
> >> 
> >> +    struct filter *filter = impl_from_IMediaStreamFilter(iface);
> >> +
> >> 
> >>      TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), ret_iface);
> >>      
> >>      *ret_iface = NULL;
> >> 
> >> @@ -191,10 +195,12 @@ static HRESULT WINAPI
> >> filter_QueryInterface(IMediaStreamFilter *iface, REFIID ri>> 
> >>          IsEqualIID(riid, &IID_IBaseFilter) ||
> >>          IsEqualIID(riid, &IID_IMediaStreamFilter))
> >>          *ret_iface = iface;
> >> 
> >> +    else if (IsEqualIID(riid, &IID_IMediaSeeking) &&
> >> filter->seekable_stream) +        *ret_iface =
> >> &filter->IMediaSeeking_iface;
> >> 
> >>      if (*ret_iface)
> >>      {
> >> 
> >> -        IMediaStreamFilter_AddRef(*ret_iface);
> >> +        IUnknown_AddRef((IUnknown *)*ret_iface);
> >> 
> >>          return S_OK;
> >>      
> >>      }
> >> 
> >> @@ -544,11 +550,74 @@ static HRESULT WINAPI
> >> filter_EnumMediaStreams(IMediaStreamFilter *iface, LONG in>> 
> >>      return S_OK;
> >>  
> >>  }
> >> 
> >> -static HRESULT WINAPI filter_SupportSeeking(IMediaStreamFilter *iface,
> >> BOOL bRenderer) +static IMediaSeeking *get_seeking(IAMMediaStream
> >> *stream)
> >> 
> >>  {
> >> 
> >> -    FIXME("(%p)->(%d): Stub!\n", iface, bRenderer);
> >> +    IPin *pin;
> >> +    IPin *peer;
> >> +    IMediaSeeking *seeking;
> >> 
> >> -    return E_NOTIMPL;
> >> +    if (FAILED(IAMMediaStream_QueryInterface(stream, &IID_IPin, (void
> >> **)&pin))) +    {
> >> +        WARN("Stream %p does not support IPin.\n", stream);
> >> +        return NULL;
> >> +    }
> >> +
> >> +    if (FAILED(IPin_ConnectedTo(pin, &peer)))
> >> +    {
> >> +        IPin_Release(pin);
> >> +        return NULL;
> >> +    }
> >> +
> >> +    if (FAILED(IPin_QueryInterface(peer, &IID_IMediaSeeking, (void
> >> **)&seeking))) +    {
> >> +        IPin_Release(peer);
> >> +        IPin_Release(pin);
> >> +        return NULL;
> >> +    }
> >> +
> >> +    IPin_Release(peer);
> >> +    IPin_Release(pin);
> >> +
> >> +    return seeking;
> >> +}
> >> +
> >> +static HRESULT WINAPI filter_SupportSeeking(IMediaStreamFilter *iface,
> >> BOOL renderer) +{
> >> +    struct filter *filter = impl_from_IMediaStreamFilter(iface);
> >> +    unsigned int i;
> >> +    HRESULT hr = E_NOINTERFACE;
> >> +
> >> +    TRACE("filter %p, renderer %d\n", iface, renderer);
> > 
> > Without tests to show whether/how the "renderer" value makes a
> > difference, I think we should print a FIXME if it's FALSE.
> > 
> >> +
> >> +    EnterCriticalSection(&filter->cs);
> >> +
> >> +    if (filter->seekable_stream)
> >> +        return HRESULT_FROM_WIN32(ERROR_ALREADY_INITIALIZED);
> > 
> > Missing LeaveCriticalSection().
> > 
> >> +
> >> +    for (i = 0; i < filter->nb_streams; ++i)
> >> +    {
> >> +        IMediaSeeking *seeking = get_seeking(filter->streams[i]);
> >> +        LONGLONG duration;
> >> +
> >> +        if (!seeking)
> >> +            continue;
> >> +
> >> +        if (FAILED(IMediaSeeking_GetDuration(seeking, &duration)))
> >> +        {
> >> +            IMediaSeeking_Release(seeking);
> >> +            continue;
> >> +        }
> >> +
> >> +        filter->seekable_stream = filter->streams[i];
> >> +        hr = S_OK;
> >> +        IMediaSeeking_Release(seeking);
> > 
> > Is it correct that only one stream is chosen for seeking, even if there
> > are multiple available? Consider e.g. the quartz filter graph, which
> > seeks all available streams.
> > 
> > For that matter, should we be delegating to the filter graph instead of
> > seeking the pins directly?
> > 
> > One thing in particular to test is whether filters are paused when
> > seeked. Even if we don't delegate to the filter graph, we likely want to
> > do this anyway.
> > 
> > Another thing to test is whether GetCurrentPosition() returns something
> > reflecting the reference clock time, or the last sample time, or whether
> > it fails if no exposed streams support GetCurrentPosition().
> 
> Sorry, on reflection, I realize this doesn't make sense, because we
> *are* the sink filter. multimedia stream ≠ stream filter...
> 
> I think the question about seeking multiple streams still stands, though.

::SetPosition tests show that only one stream is used for seeking and that 
::SupportSeeking determines which one will be used. Probably it's assumed that 
all the streams come from the same splitter filter.