[PATCH 10/25] mciseq: Limit concurrency when starting to play.

Andrew Eikum aeikum at codeweavers.com
Mon Oct 8 09:40:00 CDT 2012


That's a lot of patches! In the future, please try to send no more
than about 5 at a time, until each batch is committed. That's much
easier to review.

I reviewed the first 12, and will do the rest after those are
committed. Just reading the patches, I didn't notice any
show-stoppers.  Overall looks good to me. A couple of thoughts below.

On Thu, Oct 04, 2012 at 01:49:13PM +0200, Joerg-Cyril.Hoehle at t-systems.com wrote:
>+    switch (wmm->dwStatus) {
>+    case MCI_MODE_PLAY:
>+    case MCI_MODE_PAUSE:
>+	wmm->dwStatus = MCI_MODE_STOP;
>+    }

This is weird. You're "missing" a break, and it seems like an if
statement would be more appropriate here, anyway.

Aside from that, you introduce a scary magic number, 0x78B0. I guess
that's the MIDI sound off + control message. It would be nice if it
was a proper symbol, but native's headers don't give us MIDI symbols,
and there are a lot to define. So maybe it's not worth it. Anyway,
something to think about cleaning up in the future.

Thanks for improving this code!

Andrew



More information about the wine-devel mailing list