On 9/8/22 01:56, Rémi Bernon wrote:
+static bool async_reader_needs_wait(struct async_reader *reader, QWORD pts, DWORD *timeout) +{
- if (!reader->user_clock)
- {
REFERENCE_TIME current_time = async_reader_get_time(reader, false);*timeout = (pts - current_time) / 10000;return pts > current_time;- }
- *timeout = INFINITE;
- return pts > reader->user_time;
+}
This changes behaviour, which isn't obvious from the subject. In general please try to avoid moving code and changing it in the same patch, or if that's necessary please make it clearer in the patch subject what's being done.
+static bool async_reader_wait_pts(struct async_reader *reader, QWORD pts) +{
- IWMReaderCallbackAdvanced *callback_advanced = reader->callback_advanced;
- DWORD timeout;
- bool ret;
- TRACE("reader %p, pts %I64d.\n", reader, pts);
- if (callback_advanced && reader->user_clock && pts > reader->user_time)
- {
QWORD user_time = reader->user_time;LeaveCriticalSection(&reader->callback_cs);IWMReaderCallbackAdvanced_OnTime(callback_advanced, user_time, reader->context);EnterCriticalSection(&reader->callback_cs);- }
- while ((ret = reader->running && list_empty(&reader->async_ops)) && async_reader_needs_wait(reader, pts, &timeout))
SleepConditionVariableCS(&reader->callback_cv, &reader->callback_cs, timeout);
I can't help but feel like this callback is returning the same information in two different ways. I.e. instead of returning a bool, it could just return the timeout, and that could be zero if we are already ready to present.
I'd appreciate parentheses around "reader->running && list_empty(&reader->async_ops)".