It would be nice if you used more descriptive variable names. For example:
+struct IDirectSoundCaptureBufferImpl
+{
+ DWORD buf_size;
+ DWORD pos;
What units are these sizes and positions in? Frames? Bytes? Time? What
position is being represented here, read, write, or something else?
Using more descriptive names helps when understanding and verifying code
like this:
+ pos1 = This->pos;
+ 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;
+ }
+ else
+ pos2 = pos1;
+ pos2 %= This->buf_size;
Are all of these positions in Bytes? I hope so... (Remember this has
tripped things up in the past, see bbbf72ddcb). If everything is always
in bytes, maybe make a note of that near the variable declarations, and
then verify that it's true throughout the code. Also, why is pos1 a
separate variable here?
These patches are all missing quite a lot of error handling, which
really makes me hesitate. Especially on some important calls:
+ IAudioCaptureClient_GetBuffer(buf->cap_dev, &data, &read,
&flags, &pos, 0);
+ IAudioClient_GetDevicePeriod(This->dev, &default_period, &min_period);
+ IAudioClient_Start(This->dev);
Knowing when these functions fail will help debugging if something goes
wrong. Much worse to suddenly crash four function calls later due to
invalid default_period value than to just error out appropriately when
the error occurs.
This doesn't matter a whole lot, but I think it would be more clear to
use a separate object here, instead of a separate refcount:
struct IDirectSoundCaptureBufferImpl
{
IDirectSoundCaptureBuffer8 IDirectSoundCaptureBuffer8_iface;
- LONG ref;
+ IDirectSoundNotify IDirectSoundNotify_iface;
+ LONG ref, notify_ref;
If not, at least a comment explaining why two refcounts are needed.
Obviously a test would improve it even more, if one doesn't already exist.