On 6/10/21 7:25 PM, Zebediah Figura (she/her) wrote:
I agree with Derek's comment here. Assuming that buffering samples like this is the right thing to do, I also wonder:
should we buffer samples, or requests?
should the samples/requests be stored in a flat array instead of a
linked list? Based on usage patterns it seems that'd be easier and more efficient.
Agreed, list vs array is not really important one. Why do we need to buffer samples, is that to cover for discrepancy for already issued requests before Pause() was called? And what should really happen there, if we queue async command on RequestSample() (which is already questionable) any number of times, and then call Pause(), or Stop(), we should have a way to drop pending requests. And that's not possible with MFPutWorkItem().
With this patch any Start() when paused results in draining all the samples to the receiver, and that's not obviously correct, unless that was somehow tested. I imagine if you seek to a new position in either direction, you're not really interested in old samples. If session currently does not handle this correctly (by resetting expected samples counters, etc), we should figure out how to test it separately.
- unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0,
position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning);
- if (position->vt != VT_EMPTY)
unix_funcs->wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0,position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning);
Is this really relevant to Pause() ?
@@ -318,6 +345,8 @@ static void start_pipeline(struct media_source *source, struct source_async_comm
IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, seek_message ? MEStreamSeeked : MEStreamStarted, &GUID_NULL, S_OK, position);
flush_pending_queue(stream, TRUE); }
This will send old samples before MESourceStarted/MESourceSeeked, which is questionable.