Re: [PATCH 1/4] xaudio2: Implement IXAudio2SourceVoice frequency functions

[Chris Robinson]

On 09/15/2015 10:25 AM, Andrew Eikum wrote:
> @@ -801,14 +802,26 @@ static HRESULT WINAPI XA2SRC_SetFrequencyRatio(IXAudio2SourceVoice *iface,
>           float Ratio, UINT32 OperationSet)
>   {
>       XA2SourceImpl *This = impl_from_IXAudio2SourceVoice(iface);
> -
> TRACE("%p, %f, 0x%x\n", This, Ratio, OperationSet);
> -
> - EnterCriticalSection(&This->lock);
> -
> - This->freq_ratio = Ratio;
> - This->freq = This->fmt->nSamplesPerSec * Ratio;
> -
> - LeaveCriticalSection(&This->lock);
> -
> return S_OK;

...
> @@ -816,7 +829,16 @@ static HRESULT WINAPI XA2SRC_SetSourceSampleRate(
>       UINT32 NewSourceSampleRate)
>   {
>       XA2SourceImpl *This = impl_from_IXAudio2SourceVoice(iface);
> -
> TRACE("%p, %u\n", This, NewSourceSampleRate);
> -
> - EnterCriticalSection(&This->lock);
> -
> - This->fmt->nSamplesPerSec = NewSourceSampleRate;
> - This->freq = NewSourceSampleRate * This->freq_ratio;
> -
> - LeaveCriticalSection(&This->lock);
> -
> return S_OK;
> }

...
> @@ -2989,7 +3012,7 @@ static BOOL xa2buffer_queue_period(XA2SourceImpl *src, XA2Buffer *buf, ALuint al
>       buf->offs_bytes += submit_bytes;
>   alBufferData(al_buf, src->al_fmt, submit_buf, submit_bytes,
> -
>        src->fmt->nSamplesPerSec);
> -
>        src->freq);
>
> alSourceQueueBuffers(src->al_src, 1, &al_buf);

This doesn't strike me as correct. Assuming the SampleRate or 
FrequencyRatio can be altered in real-time, this will break the OpenAL 
buffer queue which needs to have the same sample rate on each buffer. 
SetFrequencyRatio looks like it'd map directly to the AL source's pitch 
property, while SetSourceSampleRate may be a bit trickier depending on 
when it's allowed to be called (it'd require stopping the source and 
clearing the queue, then rebuffering and requeueing the buffers to start 
where it left off).

Also, I notice the OperationSet parameter isn't used. What is that for?

[PATCH 1/4] xaudio2: Implement IXAudio2SourceVoice frequency functions

[Andrew Eikum]

On Tue, Sep 15, 2015 at 07:06:11PM -0700, Chris Robinson wrote:
> This doesn't strike me as correct. Assuming the SampleRate or
> FrequencyRatio can be altered in real-time, this will break the
> OpenAL buffer queue which needs to have the same sample rate on each
> buffer. SetFrequencyRatio looks like it'd map directly to the AL
> source's pitch property, while SetSourceSampleRate may be a bit
> trickier depending on when it's allowed to be called (it'd require
> stopping the source and clearing the queue, then rebuffering and
> requeueing the buffers to start where it left off).

Thanks again for the review! It's nice having an AL expert review my
usage; I didn't know all queued AL buffers had to have the same sample
rate. I'll look into using the pitch property, and re-queueing buffers
shouldn't be too hard, I think.
> Also, I notice the OperationSet parameter isn't used. What is that for?

It's used for syncronizing changes to different objects. Queue up
several changes with the same OperationSet, then execute all the
changes at the same time with IXAudio2::CommitChanges. I haven't
implemented that yet, and I don't see it used very often in my testing
programs.  We'll want it eventually, but it's not a priority.

Andrew