-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Always give own channel MIDI number 0 #3419
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -260,7 +260,7 @@ void CChannelFader::SetGUIDesign ( const EGUIDesign eNewDesign ) | |
UpdateGroupIDDependencies(); | ||
|
||
// the instrument picture might need scaling after a style change | ||
SetChannelInfos ( cReceivedChanInfo ); | ||
SetChannelInfos ( cReceivedChanInfo, cReceivedMIDIID ); | ||
} | ||
|
||
void CChannelFader::SetMeterStyle ( const EMeterStyle eNewMeterStyle ) | ||
|
@@ -436,6 +436,7 @@ void CChannelFader::Reset() | |
plblCountryFlag->setVisible ( false ); | ||
plblCountryFlag->setToolTip ( "" ); | ||
cReceivedChanInfo = CChannelInfo(); | ||
cReceivedMIDIID = INVALID_INDEX; | ||
SetupFaderTag ( SL_NOT_SET ); | ||
|
||
// set a defined tool tip time out | ||
|
@@ -666,10 +667,11 @@ void CChannelFader::UpdateSoloState ( const bool bNewOtherSoloState ) | |
|
||
void CChannelFader::SetChannelLevel ( const uint16_t iLevel ) { plbrChannelLevel->SetValue ( iLevel ); } | ||
|
||
void CChannelFader::SetChannelInfos ( const CChannelInfo& cChanInfo ) | ||
void CChannelFader::SetChannelInfos ( const CChannelInfo& cChanInfo, int iMIDIID ) | ||
{ | ||
// store received channel info | ||
cReceivedChanInfo = cChanInfo; | ||
cReceivedMIDIID = iMIDIID; | ||
|
||
// init properties for the tool tip | ||
int iTTInstrument = CInstPictures::GetNotUsedInstrument(); | ||
|
@@ -682,7 +684,7 @@ void CChannelFader::SetChannelInfos ( const CChannelInfo& cChanInfo ) | |
// show channel numbers if --ctrlmidich is used (#241, #95) | ||
if ( bMIDICtrlUsed ) | ||
{ | ||
strModText.prepend ( QString().setNum ( cChanInfo.iChanID ) + ":" ); | ||
strModText.prepend ( QString().setNum ( iMIDIID ) + ":" ); | ||
} | ||
|
||
QTextBoundaryFinder tbfName ( QTextBoundaryFinder::Grapheme, cChanInfo.strName ); | ||
|
@@ -886,7 +888,6 @@ CAudioMixerBoard::CAudioMixerBoard ( QWidget* parent ) : | |
bDisplayPans ( false ), | ||
bIsPanSupported ( false ), | ||
bNoFaderVisible ( true ), | ||
iMyChannelID ( INVALID_INDEX ), | ||
iRunningNewClientCnt ( 0 ), | ||
iNumMixerPanelRows ( 1 ), // pSettings->iNumMixerPanelRows is not yet available | ||
strServerName ( "" ), | ||
|
@@ -1049,7 +1050,6 @@ void CAudioMixerBoard::HideAll() | |
bIsPanSupported = false; | ||
bNoFaderVisible = true; | ||
eRecorderState = RS_UNDEFINED; | ||
iMyChannelID = INVALID_INDEX; | ||
iRunningNewClientCnt = 0; // reset running counter on new server connection | ||
|
||
// use original order of channel (by server ID) | ||
|
@@ -1123,7 +1123,7 @@ void CAudioMixerBoard::ChangeFaderOrder ( const EChSortType eChSortType ) | |
} | ||
break; | ||
case ST_BY_SERVER_CHANNEL: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this get renamed if it's always the received MIDI ID? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did wonder about that, and even started to change the name, but then had second thoughts, as it implied the menu option should change to "Sort by MIDI Channel". I think this could be more confusing to some users. The actual server channel numbers and the channel assigned to the particular client are never actually visible to the user, except when used for MIDI channels. What we are doing here by mapping is presenting to the user a view that always uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that's a definite source of potential confusion, to be honest. I'd only want my fader sorted first if I asked for it, if there's an option in the menu. I'd be wondering why the option was there, otherwise. Certainly for "No sorting", always putting own fader first is wrong unless requested. |
||
PairList << QPair<QString, size_t> ( QString ( "%1" ).arg ( vecpChanFader[i]->GetReceivedChID(), 3, 10, QLatin1Char ( '0' ) ) + | ||
PairList << QPair<QString, size_t> ( QString ( "%1" ).arg ( vecpChanFader[i]->GetReceivedMIDIID(), 3, 10, QLatin1Char ( '0' ) ) + | ||
vecpChanFader[i]->GetReceivedName().toLower(), | ||
i ); | ||
break; | ||
|
@@ -1267,7 +1267,7 @@ void CAudioMixerBoard::ApplyNewConClientList ( CVector<CChannelInfo>& vecChanInf | |
vecpChanFader[iChanID]->Reset(); | ||
vecAvgLevels[iChanID] = 0.0f; | ||
|
||
if ( static_cast<int> ( iChanID ) == iMyChannelID ) | ||
if ( static_cast<int> ( iChanID ) == pClient->GetMyChannelID() ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe stick There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The value doesn't change while connected, and it is just a simple Get method, so I don't think threads are relevant in this case. However, I see most of these uses are within a loop, so I'm happy to fetch once to a local variable within each function and use that within the loop. I'll add a commit for that shortly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done 072b2af |
||
{ | ||
// this is my own fader --> set fader property | ||
vecpChanFader[iChanID]->SetIsMyOwnFader(); | ||
|
@@ -1288,7 +1288,8 @@ void CAudioMixerBoard::ApplyNewConClientList ( CVector<CChannelInfo>& vecChanInf | |
// we can adjust the level even if no fader was visible. | ||
// The fader level of 100 % is the default in the | ||
// server, in that case we do not have to do anything here. | ||
if ( ( !bNoFaderVisible || ( ( iMyChannelID != INVALID_INDEX ) && ( iMyChannelID != static_cast<int> ( iChanID ) ) ) ) && | ||
if ( ( !bNoFaderVisible || | ||
( ( pClient->GetMyChannelID() != INVALID_INDEX ) && ( pClient->GetMyChannelID() != static_cast<int> ( iChanID ) ) ) ) && | ||
( pSettings->iNewClientFaderLevel != 100 ) ) | ||
{ | ||
// the value is in percent -> convert range | ||
|
@@ -1322,7 +1323,7 @@ void CAudioMixerBoard::ApplyNewConClientList ( CVector<CChannelInfo>& vecChanInf | |
} | ||
|
||
// set the channel infos | ||
vecpChanFader[iChanID]->SetChannelInfos ( vecChanInfo[idxVecpChan] ); | ||
vecpChanFader[iChanID]->SetChannelInfos ( vecChanInfo[idxVecpChan], pClient->ChanToMIDI ( iChanID ) ); | ||
} | ||
|
||
// update the solo states since if any channel was on solo and a new client | ||
|
@@ -1397,7 +1398,7 @@ void CAudioMixerBoard::SetAllFaderLevelsToNewClientLevel() | |
for ( size_t i = 0; i < MAX_NUM_CHANNELS; i++ ) | ||
{ | ||
// only apply to visible faders and not to my own channel fader | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != iMyChannelID ) ) | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != pClient->GetMyChannelID() ) ) | ||
{ | ||
// the value is in percent -> convert range, also use the group | ||
// update flag to make sure the group values are all set to the | ||
|
@@ -1426,7 +1427,7 @@ void CAudioMixerBoard::AutoAdjustAllFaderLevels() | |
for ( size_t i = 0; i < MAX_NUM_CHANNELS; ++i ) | ||
{ | ||
// only apply to visible faders (and not to my own channel fader) | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != iMyChannelID ) ) | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != pClient->GetMyChannelID() ) ) | ||
{ | ||
// map averaged meter output level to decibels | ||
// (invert CStereoSignalLevelMeter::CalcLogResultForMeter) | ||
|
@@ -1516,7 +1517,7 @@ void CAudioMixerBoard::AutoAdjustAllFaderLevels() | |
for ( size_t i = 0; i < MAX_NUM_CHANNELS; ++i ) | ||
{ | ||
// only apply to visible faders (and not to my own channel fader) | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != iMyChannelID ) ) | ||
if ( vecpChanFader[i]->IsVisible() && ( static_cast<int> ( i ) != pClient->GetMyChannelID() ) ) | ||
{ | ||
// map averaged meter output level to decibels | ||
// (invert CStereoSignalLevelMeter::CalcLogResultForMeter) | ||
|
@@ -1778,4 +1779,4 @@ void CAudioMixerBoard::SetChannelLevels ( const CVector<uint16_t>& vecChannelLev | |
} | ||
} | ||
|
||
void CAudioMixerBoard::MuteMyChannel() { SetFaderIsMute ( iMyChannelID, true ); } | ||
void CAudioMixerBoard::MuteMyChannel() { SetFaderIsMute ( pClient->GetMyChannelID(), true ); } |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ CClient::CClient ( const quint16 iPortNumber, | |
ChannelInfo(), | ||
strClientName ( strNClientName ), | ||
Channel ( false ), /* we need a client channel -> "false" */ | ||
iMyChannelID ( INVALID_INDEX ), | ||
CurOpusEncoder ( nullptr ), | ||
CurOpusDecoder ( nullptr ), | ||
eAudioCompressionType ( CT_OPUS ), | ||
|
@@ -789,8 +790,44 @@ void CClient::OnHandledSignal ( int sigNum ) | |
#endif | ||
} | ||
|
||
void CClient::OnControllerInFaderLevel ( int iChannelIdx, int iValue ) | ||
// Ensure user's own channel is always given MIDI offset 0, so it is the first physical fader | ||
// All channels with a client ID less than user's own will use ID+1 as their MIDI offset | ||
|
||
int CClient::ChanToMIDI ( const int iChannelIdx ) | ||
{ | ||
if ( iMyChannelID == INVALID_INDEX ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, it was suggested using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, we can choose an additional name for a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my PR. |
||
return iChannelIdx; | ||
|
||
if ( iChannelIdx == iMyChannelID ) | ||
return 0; | ||
|
||
if ( iChannelIdx < iMyChannelID ) | ||
return iChannelIdx + 1; | ||
|
||
return iChannelIdx; | ||
} | ||
|
||
int CClient::MIDIToChan ( const int iMIDIIdx ) | ||
{ | ||
if ( iMyChannelID == INVALID_INDEX ) | ||
return iMIDIIdx; | ||
|
||
if ( iMIDIIdx == 0 ) | ||
return iMyChannelID; | ||
|
||
if ( iMIDIIdx <= iMyChannelID ) | ||
return iMIDIIdx - 1; | ||
|
||
return iMIDIIdx; | ||
} | ||
|
||
// Handle received MIDI controls | ||
|
||
void CClient::OnControllerInFaderLevel ( int iMIDIIdx, int iValue ) | ||
{ | ||
// map the MIDI index to the real channel number | ||
const int iChannelIdx = MIDIToChan ( iMIDIIdx ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The client shouldn't need to worry about this. It should have been handled in the MIDI code. The MIDI code should signal this slot. |
||
|
||
// in case of a headless client the faders cannot be moved so we need | ||
// to send the controller information directly to the server | ||
#ifdef HEADLESS | ||
|
@@ -804,8 +841,11 @@ void CClient::OnControllerInFaderLevel ( int iChannelIdx, int iValue ) | |
emit ControllerInFaderLevel ( iChannelIdx, iValue ); | ||
} | ||
|
||
void CClient::OnControllerInPanValue ( int iChannelIdx, int iValue ) | ||
void CClient::OnControllerInPanValue ( int iMIDIIdx, int iValue ) | ||
{ | ||
// map the MIDI index to the real channel number | ||
const int iChannelIdx = MIDIToChan ( iMIDIIdx ); | ||
|
||
// in case of a headless client the panners cannot be moved so we need | ||
// to send the controller information directly to the server | ||
#ifdef HEADLESS | ||
|
@@ -816,8 +856,11 @@ void CClient::OnControllerInPanValue ( int iChannelIdx, int iValue ) | |
emit ControllerInPanValue ( iChannelIdx, iValue ); | ||
} | ||
|
||
void CClient::OnControllerInFaderIsSolo ( int iChannelIdx, bool bIsSolo ) | ||
void CClient::OnControllerInFaderIsSolo ( int iMIDIIdx, bool bIsSolo ) | ||
{ | ||
// map the MIDI index to the real channel number | ||
const int iChannelIdx = MIDIToChan ( iMIDIIdx ); | ||
|
||
// in case of a headless client the buttons are not displayed so we need | ||
// to send the controller information directly to the server | ||
#ifdef HEADLESS | ||
|
@@ -827,8 +870,11 @@ void CClient::OnControllerInFaderIsSolo ( int iChannelIdx, bool bIsSolo ) | |
emit ControllerInFaderIsSolo ( iChannelIdx, bIsSolo ); | ||
} | ||
|
||
void CClient::OnControllerInFaderIsMute ( int iChannelIdx, bool bIsMute ) | ||
void CClient::OnControllerInFaderIsMute ( int iMIDIIdx, bool bIsMute ) | ||
{ | ||
// map the MIDI index to the real channel number | ||
const int iChannelIdx = MIDIToChan ( iMIDIIdx ); | ||
|
||
// in case of a headless client the buttons are not displayed so we need | ||
// to send the controller information directly to the server | ||
#ifdef HEADLESS | ||
|
@@ -851,6 +897,9 @@ void CClient::OnControllerInMuteMyself ( bool bMute ) | |
|
||
void CClient::OnClientIDReceived ( int iChanID ) | ||
{ | ||
// remember our client ID received from the server | ||
iMyChannelID = iChanID; | ||
|
||
// for headless mode we support to mute our own signal in the personal mix | ||
// (note that the check for headless is done in the main.cpp and must not | ||
// be checked here) | ||
|
@@ -903,6 +952,9 @@ void CClient::Stop() | |
// disconnects the connection anyway). | ||
ConnLessProtocol.CreateCLDisconnection ( Channel.GetAddress() ); | ||
|
||
// forget our own channel ID | ||
iMyChannelID = INVALID_INDEX; | ||
|
||
// reset current signal level and LEDs | ||
bJitterBufferOK = true; | ||
SignalLevelMeter.Reset(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,8 @@ class CClient : public QObject | |
|
||
bool IsConnected() { return Channel.IsConnected(); } | ||
|
||
int GetMyChannelID() { return iMyChannelID; } | ||
|
||
EGUIDesign GetGUIDesign() const { return eGUIDesign; } | ||
void SetGUIDesign ( const EGUIDesign eNGD ) { eGUIDesign = eNGD; } | ||
|
||
|
@@ -272,6 +274,10 @@ class CClient : public QObject | |
Channel.GetBufErrorRates ( vecErrRates, dLimit, dMaxUpLimit ); | ||
} | ||
|
||
// convert between MIDI index and real channel index | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be done my the MIDI-handling code? Otherwise, it could do with a longer comment explaining why it's here. |
||
int ChanToMIDI ( const int iChanIdx ); | ||
int MIDIToChan ( const int iMIDIIdx ); | ||
|
||
// settings | ||
CChannelCoreInfo ChannelInfo; | ||
QString strClientName; | ||
|
@@ -292,6 +298,9 @@ class CClient : public QObject | |
CChannel Channel; | ||
CProtocol ConnLessProtocol; | ||
|
||
// this client's channel ID sent by the server | ||
int iMyChannelID; // must use int (not size_t) so INVALID_INDEX can be stored | ||
|
||
// audio encoder/decoder | ||
OpusCustomMode* Opus64Mode; | ||
OpusCustomEncoder* Opus64EncoderMono; | ||
|
@@ -398,10 +407,11 @@ protected slots: | |
void OnCLPingWithNumClientsReceived ( CHostAddress InetAddr, int iMs, int iNumClients ); | ||
|
||
void OnSndCrdReinitRequest ( int iSndCrdResetType ); | ||
void OnControllerInFaderLevel ( int iChannelIdx, int iValue ); | ||
void OnControllerInPanValue ( int iChannelIdx, int iValue ); | ||
void OnControllerInFaderIsSolo ( int iChannelIdx, bool bIsSolo ); | ||
void OnControllerInFaderIsMute ( int iChannelIdx, bool bIsMute ); | ||
|
||
void OnControllerInFaderLevel ( int iMIDIIdx, int iValue ); | ||
void OnControllerInPanValue ( int iMIDIIdx, int iValue ); | ||
void OnControllerInFaderIsSolo ( int iMIDIIdx, bool bIsSolo ); | ||
void OnControllerInFaderIsMute ( int iMIDIIdx, bool bIsMute ); | ||
void OnControllerInMuteMyself ( bool bMute ); | ||
void OnClientIDReceived ( int iChanID ); | ||
void OnConClientListMesReceived ( CVector<CChannelInfo> vecChanInfo ); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my general comment in the main thread. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -168,9 +168,9 @@ class CSoundBase : public QThread | |
|
||
signals: | ||
void ReinitRequest ( int iSndCrdResetType ); | ||
void ControllerInFaderLevel ( int iChannelIdx, int iValue ); | ||
void ControllerInPanValue ( int iChannelIdx, int iValue ); | ||
void ControllerInFaderIsSolo ( int iChannelIdx, bool bIsSolo ); | ||
void ControllerInFaderIsMute ( int iChannelIdx, bool bIsMute ); | ||
void ControllerInFaderLevel ( int iMIDIIdx, int iValue ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's really a "virtual channel", where 0 is our fader, and the others are allocated in server order. In MIDI-speak it's not a channel anyway, it's a controller offset. We are just using it to access a Jamulus channel. I'm happy to rename items if you have suggestions, but I still think the architecture is ok. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's the offset from the base MIDI Controller number. It's the Jamulus channel numbers that get re-ordered so that the user's own Jamulus channel is at offset 0 when the user uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, whatever. My problem with the I'm going to leave this for a while. |
||
void ControllerInPanValue ( int iMIDIIdx, int iValue ); | ||
void ControllerInFaderIsSolo ( int iMIDIIdx, bool bIsSolo ); | ||
void ControllerInFaderIsMute ( int iMIDIIdx, bool bIsMute ); | ||
void ControllerInMuteMyself ( bool bMute ); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be using
INVALID_INDEX
here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it just needs to be something like
-1
that couldn't match a real MIDI Controller offset from0
upwards. I don't mind whether we use that symbol or define another symbol for it, although I don't see the point of changing it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I said. I was told it was confusing so I changed it. I'm raising here because it was one of the first objections on my pull request, so I'm assuming it was something important.