jamulussoftware / jamulus

Jamulus enables musicians to perform real-time jam sessions over the internet.
https://jamulus.io
Other
998 stars 222 forks source link

Allow mono input device and channel selection for stereo input devices. #2344

Open pgScorpio opened 2 years ago

pgScorpio commented 2 years ago

Describe the bug

Momentarily it is impossible to use a mono input device and also impossible to select input channels for stereo devices, though there seems to be no reason why this should not be possible, since it is possible to select twice the same input channel with devices with more than two input channels. Also in some cases it would be handy to swap L/R output channels which is currently also impossible for two channel devices.

To Reproduce

Just try different devices: mono input device: Error not 2 ch. stereo input device: no input/output selection possible more than 2 channel device: all is possible.

Expected behavior

Always being able to select L/R channels for input and output. (and for mono input devices always select twice the same mono input)

Screenshots

n.a.

Operating system

All OS'es

Version of Jamulus

All versions

Additional context

Also discussed on discussion #2268

changes for v3.8.1 windows release:

Add in sound.h: (In my opinion these and some other definitions in sound.h should actually be defined in global.h since they are client, and not OS, related, but are of the same category as MAX_NUM_IN_OUT_CHANNELS which is defined in global.h)

42 #define MIN_IN_CHANNELS 1
43 #define MIN_OUT_CHANNELS 2

Chances in sound.cpp:

168 if ( ( lNumInChan < MIN_IN_CHANNELS ) || ( lNumOutChan < MIN_OUT_CHANNELS ) )

586 vSelectedInputChannels[1]  = GetNumInputChannels() > 1 ? 1 : 0;

588 vSelectedOutputChannels[1] = GetNumOutputChannels() > 1 ? 1 : 0;

Same (alike) changes also have to be done for Mac/Linux/Android sound.cpp, sound.h

Chance in clientsettingsdlg.cpp

823 if ( ( iNumInChannels < MIN_IN_CHANNELS  ) || ( iNumOutChannels < MIN_OUT_CHANNELS  ) )

I don't think there is any other impact on the GUI, but that has to be further checked/tested.

ann0see commented 2 years ago

Great thanks. If no discussion comes up the next 2 day, feel free to open a PR with these changes. I hope it doesn’t have an impact on other functionality (you never know…)

henkdegroot commented 2 years ago

should actually be defined in global.h since they are client, and not OS,

Sound.h is in the windows folder for a reason, and believe this is specifically for Windows OS only. Also the input/output selection option only exists in Windows. Linux (using JACK) is using the jack control options to select/connect and macOS is listen the different option in the "Audio Device" drop down.

EDIT: In the clientsettingsdlg.cpp, it does seem to have logic for macOS as well. I don't have a mac to test/check this. I actually wonder why these selection option should ever be hidden (apart from other OS's then windows and potentially macOS).

pljones commented 2 years ago

I actually wonder why these selection option should ever be hidden (apart from other OS's then windows and potentially macOS).

None of the top panel should even be in the client settings dialog -- it's all sound system specific and should be laid out there... Ah well. The channel selectors are not, and I agree - I've never understood why they get hidden under certain circumstances.

henkdegroot commented 2 years ago

@pljones, do you believe the channel selectors are beneficial to have on others OSs (like Linux)? Would they actually have a purpose there?

pljones commented 2 years ago

Historically, I'd expect the channel selectors were only added when ASIO support was added, to enable this choice. With Jack, you'd be expected to connect correctly using qJackCtl.

But still, if I've a stereo pair connected and Jamulus decides to connect them a certain way, it's strange that if I have more than two ins, I can adjust that but if I only have two ins, I cannot, regardless of platform. I just feel I should be allowed to set it how I want.

I've actually not come across a problem myself with the way it is now -- I use Reaper to control routing and it allows me the flexibility I need. That's because it has "more than 2" inputs, so I can adjust each client I run to use the inputs I want.

The current recommended way to handle separate inputs is to run multiple instances of Jamulus. But that requires an ASIO driver (like ReaRoute) that supports this kind of set up.

henkdegroot commented 2 years ago

With Jack, you'd be expected to connect correctly using qJackCtl.

So we should update the logic to always show the channel selection, except when JACK is used.

@pgScorpio, you happy with that? And will you do a PR for this or do you want someone else (i.e. me) to do it?

pgScorpio commented 2 years ago

@henkdegroot

Sound.h is in the windows folder for a reason, and believe this is specifically for Windows OS only.

But, as far as I can see, some definitions in sound.h are client related, not sound related, and so are (and should be) equal in all sound.h files....

... and macOS is listen the different option in the "Audio Device" drop down.

And to be consistent it would be nice if we eventually could change this to the same way of input/output selection as on other OS'es too... (I personally find this list on macOS very inconvenient!)

do you believe the channel selectors are beneficial to have on others OSs (like Linux)? Would they actually have a purpose there?

I think Yes. 1: To use one input channel of a stereo device twice to have (pseudo) mono. 2: To be able to swap L/R output channels.

And even though Jack can do this, not everyone is familiar with Jack, and in my opinion we should try to have the GUI as consistent as possible for all OS'es, so preventing all kinds of conditional code with all it's risks. And maybe, in the future, we even could consider to configure Jack from Jamulus using the input/output selection in Jamulus.

So we should update the logic to always show the channel selection, except when JACK is used.

See previous remark. So could (should?) also be valid for Jack.

you happy with that? And will you do a PR for this or do you want someone else (i.e. me) to do it?

If I can get my build environment up and running soon (for all OS'es) I will happily do the PR if you prefer that, but since it's initially just a few lines of code it would be much faster if someone more experienced with building Jamulus could already implement and test it for all OS'es.

pgScorpio commented 2 years ago

But still, if I've a stereo pair connected and Jamulus decides to connect them a certain way, it's strange that if I have more than two ins, I can adjust that but if I only have two ins, I cannot, regardless of platform. I just feel I should be allowed to set it how I want.

That's exactly my point of view...

henkdegroot commented 2 years ago

I have done some testing and looks like we should improve the "spacing" in the input/output selection section: image

As you can see in my screenshot, the important part (using input 1 or 2/output 1 or 2) is not visible. In the pull down it is possible to see (but that is because text contains some ... to make it fit.

I don't think there will be any translation which increases the space required for the L and R text.

For JACK, I don't see any added value with the current options as it just lists two options named "Default": image

pgScorpio commented 2 years ago

For JACK, I don't see any added value with the current options as it just lists two options named "Default":

Where does this "Default" come from?? It makes no sense to me. "Default" could be in the selection list, but the box should always show the actual selected input/output.

Also I think for jack it should still be possible to swap Left and Richt channels... Just for consistency. Please keep the UI the same as much as possible for all platforms.

(As soon as I'm up and running to build all OS Jamulus versions I will start working on a platform independent sound interface, and so the UI and the way of selecting inputs/outputs will be the same for all platforms too.)

ann0see commented 2 years ago

As soon as I'm up and running to build all OS Jamulus versions I will start working on a platform independent sound interface, and so the UI and the way of selecting inputs/outputs will be the same for all platforms too.

Although unrelated, it would be great if you document your work in the docs folder as a short overview

henkdegroot commented 2 years ago

Where does this "Default" come from??

I have not modified any code, just made sure the input/output selections are always shown and that is how Jack is listed. Guessing it could be the JACK server (which is named default...by default). I have not seen any application that allows the input/output to be selected within the app when using JACK. They all revert to use the Qjackctl for that.

EDIT The text "Default" comes from the soundbase.h which contains the GetInputChannelName and GetOutputChannelName function. Not exactly sure when these are used.

A platform independent sound interface....that sounds awesome :)

henkdegroot commented 2 years ago

@pgScorpio seems to me that the linux/sound.h does not contain the function to "override" the 'Default" name defined in soundbase.h. The windows/sound.h and mac/sound.h do have this functionality. Think some extra functionality needs to be implemented to "get" the available ports in JACK (after Jamulus has registered them) and list them as Input and Output.

pgScorpio commented 2 years ago

The text "Default" comes from the soundbase.h which contains the GetInputChannelName and GetOutputChannelName function. Not exactly sure when these are used.

Sounds logical that some virtual functions are not implemented for jack, since there where not used... So we should see which would have to be implemented too... probably also: `

virtual void    SetLeftInputChannel ( const int ) {}

virtual void    SetRightInputChannel ( const int ) {}

virtual int     GetLeftInputChannel() { return 0; }

virtual int     GetRightInputChannel() { return 1; }

virtual void    SetLeftOutputChannel ( const int ) {}

virtual void    SetRightOutputChannel ( const int ) {}

virtual int     GetLeftOutputChannel() { return 0; }

virtual int     GetRightOutputChannel() { return 1; }

`

A platform independent sound interface....that sounds awesome :)

Yes it will be! and it looks very doable already for Windows and macOS. but I didn't look into oboe yet and for Jack it will be challenging to get the same functionality as in the other OS'es, since it would need some JackCtl functionality in the sound module for jack. (Anyone already experience with this?) It will take some time, but I'm sure we can do it.

pgScorpio commented 2 years ago

Although unrelated, it would be great if you document your work in the docs folder as a short overview

I will, but first I wanna get all my build environments working. (Some good detailed doc's on that would be handy too. I'm still struggling. I encounter a lot of incomplete or old, no longer valid, info)

But here is a first glance at the basic idea:

Change CSoundBase to a pure virtual (interface) base class with only the functions really needed by Jamulus. So only control functions like: SetCallBacks ( CSoundCallbacks* callbacks ), ( CSoundCallbacks will be a base class of CClient, ) GetDriverProperties, (can be just flags like "do we have a driver setup dialog?") Init, Start, Stop, ...

And basic UI functions like: GetDeviceList, SelectDevice, GetCurentDevice GetInputChannelList, SelectLeftInputChannel, SelectRightInputChannel, GetLeftInputChannel, GetRightInputChannel GetOutputChannelList, SelectLeftOutputChannel, SelectRightOutputChannel, GetLeftOutputChannel, GetRightOutputChannel OpenDriverSetup

The actual CSound instance will be created in main and just it's CSoundBase interface will be passed to the CCLient constructor which should call Sound.SetCallBacks ( this ).

For windows it should be mostly moving some code around between CSound and CClient. Same for macOS, but there the CSound implementation would actually become simpler (i.e No longer generating a long list of all in/out combinations). For Linux/Jack it will be a challenge to get the full same channel selection functionality (via JackCtl?), but can probably initially be limited to just the possibility to swap Left and Right Channels or selecting twice the same input channel.

ann0see commented 2 years ago

@pgScorpio just a short question: did you succeed with generating VS files? I might be able to create them if it didn’t work for you

ann0see commented 2 years ago

It should be possible to generate them with qmake -tp vc Jamulus.pro in the x86_x64 Cross Tools Command Prompt for VS 2019

After that move the ASIO SDK into the Windows folder

pgScorpio commented 2 years ago

@pgScorpio just a short question: did you succeed with generating VS files? I might be able to create them if it didn’t work for you

Yes, thanks for asking, It took some trial and error to find out the needed Qt and VC installation options, necessary pip, aqt, choco and qmake build parameters, but I now can compile and debug with Qt Creator as well as with Visual Studio (At least on windows) Any ideas on how to automatically set custom build folders in the generated vcxproj files would still be welcome though. Also I question why there is an absolute $QtInstallPath in deploy_windows.ps1. I didn't install to C:\Qt but have all on an external SSD since I wanna use the same sources to build on different machines (also for other OS'es, so also multiple Qt installations on the same drive).

As a matter of fact, I already did a complete redesign of the ASIO implementation. No more ASIO SDK needed, Just some extra files in the windows folder (The basic ASIO definition headers from the ASIO SDK and a new asiodrivers.cpp/h and some minor changes in sound.cpp/h). ( If anyone wanna take a look at my first windows ASIO redesign feel free to comment... Maybe we should open a new discussion for that? )

But I wonder what is the best way to get things really going... Scanning through the code I encounter a lot of strange implementation decisions like why are there 2 extra inputs, just mixed together, when using Jack? Not exactly KISS since this can easily be done in Jack itself isn't it?

So should I open discussions and/or change requests for every issue I encounter? Do we do the complete sound redesign in one pull request, or do we prefer to do it in multiple steps? And what would be the best point in time to start pull requests for the audio redesign? After the 3.8.2 release maybe?

And finally I still have to get familiar with Git and get compiling and debugging going on Linux and macOS. No idea yet on how I am going to do Android. I guess I can do that on windows too? Has anyone of the Android designers any good advice?

hoffie commented 2 years ago

Thanks for working on this! Yes, post-3.8.2 should be the best time, first reviews could potentially already happen now, it's just that we can only merge such PRs after the release.

Such refactorings always carry a risk of being hard to review, so it would be good to have a good trail of what has been done. Iterative changes are way easier to review and reason about regarding potential breakage. Lots of current code might look strange, but there may be reasons to do it exactly that way in order to avoid issues. git log and git blame on the relevant files can help to see when and why the code has been written like it is now.

I suggest:

ann0see commented 2 years ago

Also I question why there is an absolute $QtInstallPath in

Although I’m not sure, but I think that’s just due to history. Maybe Autobuild doesn’t set the QT directory as parameter of the script.

Concerning custom folders – I‘m afraid I can’t help you with that since I usually don’t work with VS.

Concerning the sound api changes, yes provide small PRs to lower the review and merge barrier. Since it’s basically a heart operation there might be tough reviews and tests.

Ah yes: and please upload the files to a git repo. I‘m sure we can help you with git if you need.

ann0see commented 2 years ago

Concerning Android: I‘d ask and @j-santander @sthenos

Setting up the Android build is a pain. I think Windows is easier, but have only tried to do it in a Ubuntu docker container to stay as close as possible to the autobuild/ scripts. Probably you need to install Android Studio

And iOS: @ngocdh

pgScorpio commented 2 years ago

Thanks for all reactions. I am taking notes and will come back to the different subjects as soon as I start working on them.

I will soon open a general discussion for the redesign, or should I better open an issue instead?

I will do my first PR (for the Windows ASIO redesign) as soon as v3.8.2 is there.

hoffie commented 2 years ago

I will do my first PR (for the Windows ASIO redesign) as soon as v3.8.2 is there.

It's expected next weekend, but if you want, you can already start your work. You should base it on the latest master branch anyway and we do not expect any changes till 3.8.2 final.

pljones commented 2 years ago

Yes, you can start from master now. You'll want to get into the practice of watching Github for other people's changes being merged and deciding when enough has happened it's a good idea to rebase your work. More often means less likelihood your rebase will fail.

pljones commented 1 year ago

I'm hoping this is a fairly easy fix - just remove the check that currently prevents the fields being displayed.

pljones commented 1 year ago

(Noting that's only to allow the selectors to display -- not to make them display anything useful. That would be a separate change to keep this one focussed.)

pgScorpio commented 1 year ago

(Noting that's only to allow the selectors to display -- not to make them display anything useful. That would be a separate change to keep this one focussed.)

Not exactly... It should allow to select the same channel twice or to swap left and right channel. Very usefull for "stereo" interfaces with one mic and one instrument input. (Like Behringer UM2/UMC22, Focusrite Solo)

pljones commented 1 year ago

Yes - for that, all that's needed is allowing the selectors to display. Once displayed, they'll have their functionality enabled, I believe.

pgScorpio commented 1 year ago

Yes - for that, all that's needed is allowing the selectors to display. Once displayed, they'll have their functionality enabled, I believe.

For stereo devices yes. To allow mono devices there are some more checks and the default channel selection to be modified.