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).
Hi Chris,
Chris Robinson schreef:
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.
It's handled properly with the modulus.
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)?
Woops, mild oversight there. cbSize is allowed to be sizeof(WAVEFORMATEXTENSIBLE)-sizeof(WAVEFORMATEX), or sizeof(WAVEFORMATEXTENSIBLE) (sloppy programming), but the error checking is a lot stricter compared to WAVEFORMATEX, so this is correct.
- 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.
Theoretically you're right. I'll send an updated version.
- 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)
For bigger buffers we should probably just request up to a 500 ms buffer top, but I suppose there's no harm in wasting some extra space, as long as it's not excessive. I'll call it with buf_size / nBlockAlign for now, if an application requests a huge buffer and the code breaks, I will revisit this again. :)
Cheers, Maarten.