Code review comment for lp:~mixxxdevelopers/mixxx/features_vcman

Hey Bill,

Looking good -- here are my comments:

* In VinylControlManager, it's a little sketchy to me that SoundManager is passed in and that it's used to scan through its list of enabled inputs -- seems like VCManager shouldn't know those details of SoundManager. Would it be possible to determine which VC decks were enabled by just keeping track of onInputConnect/onInputDisconnect calls? Also naming the parameter 'deck' is a little misleading since one turntable can control multiple decks (or will in the future?) Since you're checking for a matching AudioInput index, doesn't this just test whether the N'th VC input is enabled? Anyway I would just replace this with a QSet<AudioInput> that you add to onInputConnect and remove from onInputDisconnect, that way you can just make this test whether an appropriate AudioInput is in the set.

* I don't think I understand how QSemaphore protects m_proxies in this case. QList is not thread safe so two threads should never access it at once -- this includes using QList::at, so all accesses to the list should be guarded by a monolithic QMutex (yea .. it sucks). I would use a QMutex and then make sure to unlock it the second you get the VCProxy that you need from the list (or in the case of creating the VC proxy, create it, then lock and insert it and unlock).

Otherwise everything seems fine to me -- we should have someone test it out first to verify that it actually works ;)