[PATCH v4 2/2] mf/sar: Allow requesting more than a sample per period.

[Giovanni Mascellani]

Hi Nikolay,

Il 23/06/21 23:04, Nikolay Sivov ha scritto:
> On 6/22/21 5:34 PM, Giovanni Mascellani wrote:
>> @@ -1359,7 +1369,10 @@ static HRESULT WINAPI audio_renderer_stream_ProcessSample(IMFStreamSink *iface,
>>       EnterCriticalSection(&renderer->cs);
>>       if (renderer->state == STREAM_STATE_RUNNING)
>>           hr = stream_queue_sample(renderer, sample);
>> -    renderer->flags &= ~SAR_SAMPLE_REQUESTED;
>> +    if (renderer->queued_frames < renderer->target_queued_frames)
>> +        IMFMediaEventQueue_QueueEventParamVar(renderer->stream_event_queue, MEStreamSinkRequestSample, &GUID_NULL, S_OK, NULL);
>> +    else
>> +        renderer->flags &= ~SAR_SAMPLE_REQUESTED;
>>       LeaveCriticalSection(&renderer->cs);
> Let's request only when running.

Right, done in v5.

It is not completely clear to me what should happen when the stream 
finishes, but whatever happens now I don't think my patch does worse.

>> +    if (FAILED(hr = IAudioClient_GetDevicePeriod(renderer->audio_client, &period, NULL)))
>> +    {
>> +        WARN("Failed to retrieve device period, hr %#x.\n", hr);
>> +        return hr;
>> +    }
>> +    renderer->target_queued_frames = 2 * period * samples_per_second / 10000000;
>> +
> Could this be replaced with GetBufferSize() that returns size in frames?

No, buffer and period are two different things.

The buffer length is the total amount of memory that the audio client 
reserves for receiving data from the caller. The period is how often the 
audio client processes the incoming data. Usually the buffer is about 
three periods long.

In my understanding the audio client runs something like this loop:

while (1)
{
     fetch_a_period_of_data_from_the_buffer();
     send_data_to_audio_card();
     SetEvent(event);
     Sleep(period);
}

So, it is important that every time the event is set, at least a period 
is written to the buffer. Maybe even more, if there is space, to be sure 
we are not going to miss the next round. This is what my patch does: if 
the queue has less than a period (actually two, again to be sure) of 
data, it loads more without waiting for the event.

In a sense, at every round we have to process at least a period of data 
and at most a buffer of data (because it is impossible to write more 
than a buffer; actually, more than buffer - padding, as my 
already-accepted patch fixed).

>> @@ -1789,6 +1812,7 @@ static void audio_renderer_render(struct audio_renderer *renderer, IMFAsyncResul
>>                                       IAudioRenderClient_ReleaseBuffer(renderer->audio_render_client, dst_frames, 0);
>>   
>>                                       obj->u.sample.frame_offset += dst_frames;
>> +                                    renderer->queued_frames -= dst_frames;
> queued_frames should be reset when every time we empty this list.

Right, done in v5.

> Have you tested patch v4 with the game?

Yes, and also v5. Stuttering is fixed in all the three games mentioned 
in the description.

Thanks, Giovanni.