http://bugs.winehq.org/show_bug.cgi?id=33045
--- Comment #18 from Jörg Höhle hoehle@users.sourceforge.net 2013-03-09 04:31:10 CST --- Created attachment 43862 --> http://bugs.winehq.org/attachment.cgi?id=43862 patch: Fix notification when recording using MSACM.
The data is equally consistent with samples = bytes * nSamplesPerSec / nAvgBytesPerSec; // not MulDiv
Your patch looks ok now, yet I still have a suggestion. In WOD_PushData, it's important that both the while and for loop compute the same number of frames. To ease the job of your poor reviewer, it would be nice to have both use the same function, e.g. WINMM_HeaderLenBytes (BTW, I don't like having three tiny functions do almost the same thing).
Here's a patch to fix notification in WID_PullACMData atop your patch. I plan to rebase it against 1.5.25 and submit it on Monday, so that it does not come in the middle of your patch series.
What still needs fixing? - GetPosition returns bogus numbers with ACM codecs, e.g. wave.c:1626: Test failed: after position: 15655 samples wave.c:1632: Test failed: after position: 4007680 bytes - not * 256
- Think about WHDR_ENDLOOP without preceding BEGIN. Wine will hang or crash.
- MarkDoneHeaders is still bogus w.r.t. LOOP, see comment #3
- WID_PullACMData looks incomplete. It starts with checking device->acm_hdr as if it could persist from a prior invocation, however it frees it when returning normally. Thus it should remain local to the function, with device->acm_offs. In error situations, the current code may break, because it may see device->acm_hdr.cbDstLength != 0 on entry from a prior acmStreamConvert MMSYSERR_* return. Hmm, now that my patch eliminated the tail call, it could as well fix that.
- WID_PullACMData needs better error handling. If acmStreamConvert returns an error, the IACaptureClient buffer remains locked.
- In WID_PullACMData, the queue->lpNext condition looks bogus in the light of IMA_ADPCM's 256 bytes block size. More generally, PullACMData needs a redesign. What to do when srcLengthUsed < packet_frames ?!? I.e. when the mmdevapi packet length does not match the codec's blocksize, e.g. using 10ms packets while IMA_ADPCM likely needs multiples of 256 bytes. - We should not throw away recorded bytes given enough buffers. - mmdevapi does not accept retrieving less than one packet... Ouch, that's enough material for a separate bug report.