So the right thing to do seems to wait for the currently in-flight sound to be played. A pre-requisite problem I found on the way is what is addressed in the first patch ("Do not limit maximum fill when clearing the buffer with zeroes."). The overall dsound buffer is 1sec long. Regardless of my changes 'send_sample_data(filter, -1, NULL, filter->buf_size);' in render_thread_run() is currently waiting about 1s always. That's because of maximum fill check in get_write_pos() which, once 150ms of zeroes are filled into the buffer, will wait until those are played before moving further. While what we probably want to do here (especially with my patch addressing EC_COMPLETE event timing) is to only wait for the previously submitted data not yet played. That is achieved with the first patch, it will now wait only for the amount of data previously present in the buffer.
I'm not thinking through things as clearly as I'd like (and dsoundrender really demands it), but as long as we're not overwriting old data (and I think we aren't) then I don't think there's a reason to ever have that limit? I think we can just remove it entirely.
The second tests patch (Check for EC_COMPLETE more often in test_eos()) helps to interpret test results, currently 'todo_wine' appear after later calls while EC_COMPLETE was sent from earlier calls. The added todo with "Send all queued samples to dsound before issuing EC_COMPLETE." is not really added, that is essentially pre-existing extra EC_COMPLETE (removed todo before that) which is arriving later after the patch. All the remaining EC_COMPLETE-related todos are fixed in the last patch (which is a separate aspect of handling flush, which is not exactly related to the other parts).
Fine, although some of those really are redundant, but eh, it's not like they're doing any harm.
3/4:
``` @@ -361,14 +377,18 @@ static DWORD WINAPI render_thread_run(void *arg) { IMediaSample *sample;
- if (filter->eos) + if (filter->eos && (filter->filter.state != State_Running || !filter->queued_sample_count)) { + BOOL complete_pending; + + complete_pending = filter->eos_complete_pending; + filter->eos_complete_pending = FALSE; LeaveCriticalSection(&filter->render_cs); - TRACE("Got EOS.\n"); /* Clear the buffer. */ send_sample_data(filter, -1, NULL, filter->buf_size); - - TRACE("Render thread exiting.\n"); + if (complete_pending) + complete_eos(filter); + TRACE("Render thread exiting on EOS.\n"); return 0; } ```
So this is actually kind of two changes:
(1) Don't clear the buffer until we've played all the queued samples. That's fine, except I don't think we want the exception for paused state?
(2) Delay complete_eos(). This is... yeah, as you mentioned, the right time, at least after 1/4. It's not the most declarative way to do it—we really should be waiting for the right playpos—but I'll accept it.