[PATCH 4/5] dsound: Add support for OpenAL IDirectSoundCaptureBuffer

Chris Robinson chris.kcat at gmail.com
Mon Nov 30 21:57:17 CST 2009


Hi.

On Monday 30 November 2009 1:56:20 pm Maarten Lankhorst wrote:
> +    if (This->playing)
> +    {
> +        pos2 = This->format->nSamplesPerSec / 100;
> +        pos2 *= This->format->nBlockAlign;
> +        pos2 += pos1;
> +        if (!This->looping && pos2 > This->buf_size)
> +            pos2 = 0;
> +        else
> +            pos2 %= This->buf_size;
> +    }

Does non-looping capture let the write position get to This->buf_size? It 
looks like a position of This->buf_size should set the position to 0 too, if 
This->buf_size+1 would.

> +        if (format->wFormatTag == WAVE_FORMAT_EXTENSIBLE)
> +        {
> +            WAVEFORMATEXTENSIBLE *wfe = (WAVEFORMATEXTENSIBLE*)format;
> +            if (!IsEqualGUID(&wfe->SubFormat,
> &KSDATAFORMAT_SUBTYPE_IEEE_FLOAT))
> +                return DSERR_BADFORMAT;
> +            else if (format->cbSize < sizeof(WAVEFORMATEXTENSIBLE)-
> sizeof(WAVEFORMATEX))
> +                return DSERR_INVALIDPARAM;
> +            else if (format->cbSize > sizeof(WAVEFORMATEXTENSIBLE)-
> sizeof(WAVEFORMATEX)
> +                && format->cbSize != sizeof(WAVEFORMATEXTENSIBLE))
> +                return DSERR_CONTROLUNAVAIL;
> +        }

You should probably check the cbSize before accessing the extended 
information, otherwise it may indicate that there isn't enough space for the 
extended information. Also, is it necessary to reject formats with a higher 
cbSize than needed (IIRC, Wine's tests showed some Windows versions allowed 
it)?

> +    buf_format = alGetEnumValue(fmt_str);

Unfortunately, you can't call this function when a context is not set (it'll 
probably work on some systems, but technically shouldn't). And setting a 
context makes no sense for capture. Setting buf_format directly to the enum in 
the switches should be all you need to do.. just make sure to initialize it to 
0 so the check won't use it uninitialized.

> +    This->dev = alcCaptureOpenDevice(This->parent->device,
>  This->format->nSamplesPerSec, buf_format, This->buf_size);

Note that the buffer size given to OpenAL is in sample frames, not bytes. It's 
not a direct problem here, per se, but it may be an issue if a large size is 
requested (ie, where n samples would work, but n*2 or n*4 won't).



More information about the wine-devel mailing list