[PATCH 2/2] mf/sar: Request a new sample immediately if the buffer is too empty.

[Nikolay Sivov]

On 6/22/21 1:12 PM, Giovanni Mascellani wrote:
> Hi,
>
> Il 22/06/21 10:35, Nikolay Sivov ha scritto:
>> How SAR handles that on Windows? Could we simply adjust a number of
>> requests that sink sends, based on "audio client buffer size" /
>> "incoming sample buffer size".
>
> I don't know how Windows' SAR does this. Is there I way I can
> instantiate Windows' SAR using an IAudioClient provided by me? It
> seems that it will only take device from the MMDeviceEnumerator
> provided by Windows itself.
Yes, it's probably too hard to experiment with that in a clean way.
>
> The incoming sample buffer size is not known in advance, and it can
> vary a lot even within the same stream. It can go from 128 to 1024
> frames at least. I guess it is a byproduct of how the decoder works
> and how packets are laid in the encoded file, as well as of the
> internals of any other transform in the media session. I don't know
> much about how Vorbis, but I guess we shouldn't depend on any
> assumption on how large the samples are: if we receive small samples,
> we just have to ask for more until our playback buffer is full enough.
Can we check for "maxframe - pad_frames < accumulated-so-far +
some_delta", and request more? Or maybe don't even need to check buffer
sizes at all, we initialize with predetermined length, so we can request
as much as we need until we have this duration worth of data. I just
don't see why we should ignore event-based rendering, after explicitly
requesting it from mmdevapi.
>
>> I don't think you should touch this event yourself, when it's supposed
>> to be signaled by the audio system.
>
> I don't think it is a problem if we touch that event. From my
> understanding it's not something that IAudioClient is supposed to use
> internally. It's just a way for IAudioClient to signal that it is
> ready to process a buffer. If we signal more often than that it's not
> an issue for the IAudioClient and certainly not for the event itself.

>
> I can implement the same change without manually signalling the event,
> though. The reason I did it this way is that it mostly fit the current
> SAR architecture, so it doesn't require a lot of changes.
I don't think reusing event for that is a good idea, it's not clear to
me if we should trigger rendering on ProcessSample() at all, or simply
ask for more samples.

Doesn't matter if it's more changes at the end, if it will fit better.
>
> BTW, as I write you I realize that my patch is even too much, I am
> sending an updated version (that still fixes the bug on Borderlands 3,
> Deep Rock Galactic and Mutant Year Zero). This revision is still
> event-based, but if you insist on a different solution I'll rework it.
>
> Basically the only important point is that when a new sample is
> received the queue is immediately processed, and not just once per
> IAudioClient period. Manually signalling the event seems to me the
> easiest and cleanest way to have that, but if you don't like it I can
> implement it another way.
>
> Thanks for the review, Giovanni.
>