Merge lp://qastaging/~mixxxdevelopers/mixxx/features_external_mixer into lp://qastaging/~mixxxdevelopers/mixxx/trunk

Just tried to test the branch on OS X, but segfault...

Debug: [Main]: Opened PortAudio stream successfully... starting
Assertion failed: (sizeof( UInt32 ) == sizeof( long )), function ringBufferIOProc, file src/hostapi/coreaudio/pa_mac_core.c, line 1713.

seems strange.. was this on bootup? or after you selected a microphone
input?

On Sat, Mar 19, 2011 at 11:43 AM, RAFFI TEA <email address hidden>wrote:

> Just tried to test the branch on OS X, but segfault...
>
> Debug: [Main]: Opened PortAudio stream successfully... starting
> Assertion failed: (sizeof( UInt32 ) == sizeof( long )), function
> ringBufferIOProc, file src/hostapi/coreaudio/pa_mac_core.c, line 1713.
>
> --
>
> https://code.launchpad.net/~mixxxdevelopers/mixxx/features_external_mixer/+merge/53747
> You proposed lp:~mixxxdevelopers/mixxx/features_external_mixer for merging.
>

> seems strange.. was this on bootup? or after you selected a microphone
> input?
>
Yes, it happens on bootup.

Hey RJ,

Here's some comments on your code:

• CircularBuffer:
  ∘ Destructor should have: delete [] m_pBuffer;
  ∘ write() comment refers to non-existent numSamples variable
  ∘ skip() - Can the while loop just be replaced with:
     ‣ m_iReadPos = (m_iReadPos + itemsToRead) % m_iLength;
  ∘ If the compiler doesn't make m_iWritePos and m_iReadPos 8-byte aligned, are you sure operations are still atomic? (Do you need a __declspec(align(8)) in there?)
• Preferences:
  ∘ Good, I like that we're passing more of these manager classes into the preferences dialog. This will make tons of sense in the long run.
• EngineMicrophone::receiveBuffer()
  ∘ Assumes the input is stereo. It should check the AudioInput passed to it first maybe?
• Re: SoundManager, your comment that says "// What should channelbase be?"
  ∘ ChannelBase is the channel on the soundcard that we have to open, with the lowest index on the soundcard. For example, if we want to open channels 3-4 and 7-8, channel 3 would be the ChannelBase. (This is relevant because in reality, we're forced to open all channels 3 through 8 contiguously for cases like this.)
• Talkover:
  ∘ Talkover on a physical mixer lowers the music volume some amount (quite a bit). I don't see that in your code anywhere, but maybe I missed that.
• mixxx.cpp
  ∘ This is the best place for EngineMicrophone to live?

I'm surprised that PortAudio lets me open two channels on my mic. It's a mono microphone, though I'm not sure if the magic is going on in CoreAudio or PortAudio to present it as two-channels. (It may be an issue on other platforms if microphones only show up as single-channel devices.)

Anyways, overall very good! Many people will be pleased by the addition of microphone talkover support.

Thanks,
Albert

I tested on Ubuntu 10.10 (32-bit) and Mac OS X 10.6 (64-bit) and I could not reproduce Tobias' ASSERT.

Tobias, how recent is your version of PortAudio?

I use the MacPorts version of PortAudio which I have never updated over the time -- my MacBook Pro (2010) is 3 months old.

I've also deleted the config file but no effects.

If an update to another MacPorts version does not help, I compile for my own. And if it is still not working after, I don't know...

Also I'm interested if Jus experience same problems?

Thanks,

Tobias

My problem seems to be known:

http://music.columbia.edu/pipermail/portaudio/2006-April/005433.html

Solved the ASSERT myself.

The problem is that Mixxx's default samplerate is 48k. Resetting to 44k resolves the ASSERT on my MacBook Pro (2010 model), iCore 5.

Note that I used the build-in microphone input as well as the build-in soundcard as master out.

Thus, line-out and microphone input may have different maximum samplerates. Don't know if one can determine the max samplerate of the mic-input --> Needs fixing somehow :-)

Also, tried to speak through the mic while I was recording and playing a track --> The recorded file does not contain my voice. Will voice be only supported if one use shoutcast?

Your voice should end up on the recording since it uses the master output...
hmm. Did you skin-and-hold the talkover button?

On Mon, Mar 28, 2011 at 10:53 AM, RAFFI TEA <email address hidden>wrote:

> Also, tried to speak through the mic while I was recording and playing a
> track --> The recorded file does not contain my voice. Will voice be only
> supported if one use shoutcast?
> --
>
> https://code.launchpad.net/~mixxxdevelopers/mixxx/features_external_mixer/+merge/53747
> You proposed lp:~mixxxdevelopers/mixxx/features_external_mixer for merging.
>

Skin-and-hold the talkover button? No ...

Could you provide some more guidance, please? Thanks :-)

Thanks RJ for your guidance on IRC.

It works fine. Just one thing: I hear myself while speaking through the mic. This is not surprising but is it possible to define an option such that any voice will go to shoutcast and not to the master out? I am not used to hear my self and it holds me off to concentrate on moderating a show :-)

Ahh hm, I think this is technically difficult because both recording and Shoutcast are exact mirrors of the master out, and that isn't configurable. Don't club DJs who use a mic have to get used to hearing themselves?

I think you're right.

 It occurs to me that EngineMicrophone is a specialization of the more general pattern of "Take samples from AudioInput, mix them into the output". I'd like to generalize it, but not now. Maybe once I or somebody else does an input passthrough. The only thing microphone-specific here is the button name "talkover".

 CircularBuffer:

So CircularBuffer isn't thread safe.. it needs memory barriers I believe. I wonder if we should steal this from PortAudio? Given the current SoundManager setup, EngineMicrophone is never touched by different threads, so this isn't a problem right now. I think I'll add a giant warning.

  ∘ Destructor should have: delete [] m_pBuffer;
Done

  ∘ write() comment refers to non-existent numSamples variable
Fixed

  ∘ skip() - Can the while loop just be replaced with:
     ‣ m_iReadPos = (m_iReadPos + itemsToRead) % m_iLength;

Sorta -- except you have to make sure the read pointer doesn't pass the write pointer. I thought there was too much of a chance I'd screw it up, so I just do it as a loop.

  ∘ If the compiler doesn't make m_iWritePos and m_iReadPos 8-byte aligned, are you sure operations are still atomic? (Do you need a __declspec(align(8)) in there?)

Probably -- as I mentioned above I'm going to stop claiming this is thread safe.

• Preferences:

• EngineMicrophone::receiveBuffer()
  ∘ Assumes the input is stereo. It should check the AudioInput passed to it first maybe?

OK, now I verify that the input type is a microphone input and that the input # of channels is 1.

• Re: SoundManager, your comment that says "// What should channelbase be?"
  ∘ ChannelBase is the channel on the soundcard that we have to open, with the lowest index on the soundcard. For example, if we want to open channels 3-4 and 7-8, channel 3 would be the ChannelBase. (This is relevant because in reality, we're forced to open all channels 3 through 8 contiguously for cases like this.)

• Talkover:
  ∘ Talkover on a physical mixer lowers the music volume some amount (quite a bit). I don't see that in your code anywhere, but maybe I missed that.

Nope, it's not there currently. Sean is saying that in general he thinks the DJ should control the microphone levels manually. He said that is how e.g. the DJM-500 works. If I do implement the talkover damping, I think it would need to be a preference option.

• mixxx.cpp
  ∘ This is the best place for EngineMicrophone to live?

It doesn't live here (i.e. isn't a member variable of MixxxApp) since EngineMaster takes ownership of it and deletes it later. It needs to be registered with SoundManager though. Basically, I didn't see a way to do it without introducing a dependency of SoundManager on EngineMicrophone, or EngineMaster on EngineMicrophone.

I could make a MicrophoneManager class which takes both the EngineMaster and SoundManager, and then hooks up the EngineMicrophone, but that seemed overkill to just get those 4 lines out of MixxxApp.

Wanted to test it. If I enable microphone input at preferences, mixx crashes. What skin I have to use?

> Wanted to test it. If I enable microphone input at preferences, mixx crashes.
> What skin I have to use?

If Mixxx does not start, please delete mixxx.cfg. Start Mixxx again and set samplerate to 44100 Hz.
This has worked for me.

Hi Martin,

What version did you test it with? Can you post the output of "bzr revno"?

Also, could you please post a backtrace? http://www.mixxx.org/wiki/doku.php/creating_backtraces

Thanks,
RJ

Revision 2560 - Ubuntu 10.10 - Thinkpad internal audio (Intel ALC269)
Output:
Debug: [Main]: Displaying mixxx
Debug: [Main]: Running Mixxx
Fatal: []: ASSERT: "false" in file src/engine/enginemicrophone.cpp, line 103

If I remove the check at lines 98 - 104 all runs fine.

But I still don't know where I can test the microphone input because I have no buttons in the skin!?

Hey Martin,

Thats' right, there's no skin yet. (this branch just adds the features
internally, and our lead artist will make skins that have them). You can
hack it by opening the "skin.xml" file for the skin you are using and
searching for "[Channel1],beatsync" and replacing every occurrence of it
with "[Microphone],talkover". Then the left sync button will act as your
microphone talkover button. You can also replace the "[Channel1],VuMeter"
references to "[Microphone],VuMeter" and "[Channel1],PeakIndicator" with
"[Microphone],PeakIndicator" if you want the left VU and peak indicators to
actually be for the microphone.

Thanks,
RJ

On Fri, Apr 1, 2011 at 11:51 AM, Martin Simon <email address hidden> wrote:

> Revision 2560 - Ubuntu 10.10 - Thinkpad internal audio (Intel ALC269)
> Output:
> Debug: [Main]: Displaying mixxx
> Debug: [Main]: Running Mixxx
> Fatal: []: ASSERT: "false" in file src/engine/enginemicrophone.cpp, line
> 103
>
> If I remove the check at lines 98 - 104 all runs fine.
>
> But I still don't know where I can test the microphone input because I have
> no buttons in the skin!?
> --
>
> https://code.launchpad.net/~mixxxdevelopers/mixxx/features_external_mixer/+merge/53747
> You proposed lp:~mixxxdevelopers/mixxx/features_external_mixer for merging.
>

Tried another pc now. Made necessary changes to the skin. Ubuntu Maverick x64 build. (could it be a 64bit problem?) Tried different hardware combinations. (M16DX in/out; Aureon 7.1 usb in/out; nvidia internal sound in/out).
I still commented out the check that runs into the assert. Most combinations of in and out fail and mixx crashes when trying to select inputs.

Most comprehensible way: Using only mainboard sound with jack. If I enable input in mixxx, jack shows the portaudio in port an connects the right input channel with it. I also can connect the input directly with the output with jack an it works. Yes, jack can route the sound from the microphone to the output and it works. The input is surely ok. But with pressing my talkover button in mixxx nothing happens. The vumeter displays nothing an no sound is routed through.

Thanks a lot for the testing Martin. It was definitely broken.

Can you pull the latest changes (and re-enable the asserts, they should only fire if something is wrong) and give it another try?

Thanks,
RJ

Tested it again and it works as suspected but .... if I release the talkover-button the last samples seem to be delivered again and again ... the sound in the microphone channel "hangs".

Ah yes, another bug -- should be fixed now.

Thanks,
RJ

On Sun, Apr 3, 2011 at 8:33 AM, Martin Simon <email address hidden> wrote:

> Tested it again and it works as suspected but .... if I release the
> talkover-button the last samples seem to be delivered again and again ...
> the sound in the microphone channel "hangs".
> --
>
> https://code.launchpad.net/~mixxxdevelopers/mixxx/features_external_mixer/+merge/53747
> You proposed lp:~mixxxdevelopers/mixxx/features_external_mixer for merging.
>