Redesign of all Sound related code

[pgScorpio]

@corrados @ann0see @henkdegroot @hoffie @pljones @ngocdh @j-santander @sthenos
( This discussion already started in this issue, and I am already working on it ;=) )

General Idea:
The same user interface for all platforms and sound interfaces (At least as much as possible.).
Remove any sound interface dependent code from the Client implementation.
All interface dependent code should be in (or accessed via) CSoundBase (the standard Sound interface, which is the same for all platforms)
So any changes in the Client software should never affect any sound interface dependent sound code and we should have far less chance on introducing platform dependent bugs in other code than in the platform folder itself.

To accomplish this:
All CSound implementations should be alike as much as possible for all platforms/interface types.
All CSound implementations should have the same interface to CClient (Just CSoundBase).
User interface should be the same, as much as possible, for all platforms/interfaces.
Any platform dependent UI options (visibility of UI controls, like the availability of Device settings (show button) or extra Interface Settings (extra tab in Settings) and i.e. WhatsThis texts should be obtained from the CSound object via CSoundBase.
Any platform/interface specific command line parameters should be read and handled by the CSound class.
Any platform/interface dependent settings should be on a separate Settings 'Interface Settings' tab which should be implemented in the platform CSound. (I am not aware of any platform/interface dependent settings yet, but we should be future proof.)

Proposed/Needed implementation steps:

• Implementation of Refactoring: Move sound sources into src/sound folder #2575 (Strongly related to Sound, and we don't wanna intruduce new files we will all have to move again later.)
• Implementation of Implementation of global messages #2598 (Used by the new CSoundBase that will handle almost all Sound related messages.)
• Implementation of Adding global commandline parameters #2599 (Each CSound will read it's applicable commandline parameters itself.)
• Implementation of Connection status client and gui can get out of sync. #2519 (The new Sound implementation will increase the risk of ui getting out of sync because it solves Don't insist that selected audio device is working #1378 and various other issues.)

After this we can do the only big merge:

• Merge of ALL new Sound files (Still compatible with the old implementation, but because of a changed CSoundBase we have to merge all CSound files for all platforms at once!)

Then, in arbitrary order: (These could each be one or more PR's each with multiple commits.)

• Remove commandline parameter passing via constructors.
• Changing CClient to use CSoundBase 'running' detection.
• Remove "input boost" from CClient and use the CSound implementation. (This will also enable separate Left and Right channel Boost, so we should already use the new CSoundProperties class for this.)
• Changing CClient to use new CSoundBase device selection functions (Also moving a lot of error handling to CSoundbase)
• Make use of the new CSoundStopper class to temporarily stop and restart Sound where still needed. (Though this will seldom be necessary anymore, since in most cases the new CSoundBase will handle the Stop-Start automatically.)
• Changing CClient and CLientDlg to use the new CSoundBase functions. (Replacing all calls to 'compatibility' functions.)
• Changing CClientDlg to use the new CSoundProperties class. (All Sound dependent ui differences like showing/hiding controls and and different texts will now be defined by the Sound class.)
• Final cleanup (Removing all backwards compatibility from CSoundbase and fix any missed needed changes in the rest of the code.)

After that we can think about the next steps:

• Final optimizations in CSoundBase/CSound's after removing the compatibility code.
• All Interface related settings should be read and saved by the CSound implementation (Depends on new Settings implementation and will need an update of the ini file structure.)
• Jack could implement "Device Settings" that would open QJackCtl if QJackCtl is installed.
• Jack could also implement a Device Setup dialog ('Jack Settings') to, at least, select a Jack patch bay file.

[ann0see]

Sounds good to me. This will not change any functionality I think? Just GUI?

[pgScorpio]

Sounds good to me. This will not change any functionality I think? Just GUI?

Indeed. No functionality changes. Only step 5 would add some functionality for Jack.
And for mac the way of channel selection changes, so that would have impact on the saved settings in the ini file.
The rest is primarily just moving code around...

EDIT:
There will be some changes, since we will solve #1378 and #2344, so visibility of controls will change in some occasions.
Also new functionality will become available, like separate input boost selection for left and right input channels and the "added channels" functionality, currently only implemented for the Windows version when exactly 4 input channels are available, will be possible for all versions whenever more than 2 input channels are available.

[pljones]

4: All Interface related settings should be read and saved by the CSound implementation.
(I think we really should create separate sections under in the ini file ? i.e , , , , ,, ,...), so every control can read and write its own settings, but that's another issue...)

I've a complete overhaul of settings handling in progress.

[pgScorpio]

I've a complete overhaul of settings handling in progress.

Any more info on your ideas? I have some ideas (from experience) myself, so, if you wish, I can contribute to that too.

[pljones]

All the code - it's in my tree.

[ann0see]

I think it's here: https://github.com/pljones/jamulus/tree/feature/next-big-thing-for-settings

[pgScorpio]

I think it's here: https://github.com/pljones/jamulus/tree/feature/next-big-thing-for-settings

OK, found it. I'll have a look...

[pgScorpio]

@pljones

I've a complete overhaul of settings handling in progress.

Could we please get in contact about that subject ?!
I have several ideas on which directions we could be going, but I'm afraid we are still proceeding in the wrong direction...

[pgScorpio]

I created a new branch for the sound redesign that can be found here.

Changes up to now:
1: I added my (heavily modified) Visual Studio files which now everybody should be able to use. See JamulusWin-README.md.
2: I replaced ASIOSDK2 by my own implementation in te windows folder.

Please have a look and comment (I'm still a newbie on GitHub, and it's my first actual change to the jamulus code, so I'm not sure everything is according normal procedures and rules.)

[ann0see]

Great news! I think it makes sense to open a Pull Request and continue discussions there. Concerning the VS changes: I think they should be handled somewhere else, but for now I think they're ok in a DRAFT/WIP (!) Pull request. As soon as a way is found to improve the VS file generation, we could put scripts somewhere.

[hoffie]

2: I replaced ASIOSDK2 by my own implementation in te windows folder.

I've had a quick look at the branch. Can you elaborate on the ASIOSDK2 implementation? At first glance, I'm seeing Steinberg-owned code which I don't think we are allowed to distribute. I'm also seeing code which seems similar to Steinberg's code in file name, comments and calls (with differences in the details, though), but doesn't carry Steinberg's Copyright header. This might be interpreted as modifying Steinberg-owned code which could be really problematic.

What's the reason for changing that ASIO interface part?

[pgScorpio]

I think it makes sense to open a Pull Request and continue discussions there.

PR opened ! (I hope I did it right ;=) )

Concerning the VS changes: I think they should be handled somewhere else, but for now I think they're ok in a DRAFT/WIP (!) Pull request. As soon as a way is found to improve the VS file generation, we could put scripts somewhere.

As long as moc and ui file generation is not changed (but just adding/removing normal source files.), these Visual Studio files do not need to be (and should never be) regenerated and normal file additions/deletions can be done in Visual Studio. These project files are fully relocatable since they do no longer contain absolute paths. So these files can now be handled as the Jamulus.pro version for Visual studio as long as they are opened via the PowerShell script. See the JamulusWin-README in my repo.

I've had a quick look at the branch. Can you elaborate on the ASIOSDK2 implementation? At first glance, I'm seeing Steinberg-owned code which I don't think we are allowed to distribute. I'm also seeing code which seems similar to Steinberg's code in file name, comments and calls (with differences in the details, though), but doesn't carry Steinberg's Copyright header. This might be interpreted as modifying Steinberg-owned code which could be really problematic.

I don't think this should be a problem since it is not really different from including the full ASIOSDDK2.
I just used the by Steinberg supplied ASIO definition headers (just moved them to the windows folder, and did some minor fixes. AFAIK I did not remove any Copyright notices!). and the other code is a structured replacement of the spaghetti ASIOSDK2 example code and some modified Jamulus specific code.

Also see the Steinberg ASIO 2.3.3 Licensing Agreement 2.0.1 - 2019.pdf (maybe we should include that one in the repo too?)

"The Licensee is only allowed to publish an ASIO Driver Compliant Product and/or ASIO Compliant Hardware Products to the public based on a Licensed Software Developer Kit which is not declared as a preview or beta version by Steinberg"

[hoffie]

Heh, I had written my comment before the PR was open. I think it now makes sense to continue that discussion in the PR: #2407 (comment)

[jujudusud]

Hi @pgScorpio , are you interested to dig into the implementation of Wasapi on windows version of Jamulus?

[ann0see]

@npostavs did work on That with port audio. See the respective feature branch.

[jujudusud]

I know. Some help will certainly not been refused, isn't it ?

[pgScorpio]

Hi @pgScorpio , are you interested to dig into the implementation of Wasapi on windows version of Jamulus?

Sure, once this PR is completed...

[38github]

Would "pure" Pipewire support not going through JACK be an improvement?

[pgScorpio]

Just triggering this discussion again....

It is now two months later and the sound-redesign is waiting on #2575, #2598, #2599 and #2519
In the mean time it would be useful to continue this discussion since also #2496, #2344, #2238, #1378 and probably even more issues should/could all be solved with the sound-redesign too.

So comments on all of these these, and also #2407 and #2590, are still very welcome to be discussed here.
Also new ideas on how to proceed with implementation of the sound-redesign are very welcome....

[ann0see]

Thanks for all your work so far ;-).

Yes, I fully agree that we should continue discussion. However, it's quite difficult to continue if we don't see exact points to discuss ;-).
Can you convert your list above to a checklist style (what you plan to do, what exactly needs discussion,...). I think #2503 is a good example on how this could look like. Some PRs are stuck at the moment - and I'm unsure why. I think we're missing a second reviewer.

[pgScorpio]

Can you convert your list above to a checklist style

Done. Is this what you would expect it to be ?

[ann0see]

Yes. That's good

[ann0see]

Concerning the proposed changes: I think

First redesign all CSound implementations so they are structured in the same way as much as possible for all platforms. No Client changes yet

Can be agreed on (I've read it many times by @pljones and also stand behind making it as uniform as possible)

First redesign of CSoundBase definition so it potentially can accommodate any Sound implementation. (Change platform CSound's and Client code accordingly.

Can you elaborate? What exactly are you planning?

All Interface related settings should be read and saved by the CSound implementation.

I think that's somewhat a farther away, independent goal? Could potentially clash with the settings redesign by pljones.

Jack should also implement a Device Setup dialog ('Jack Settings') to, at least, select a Jack patch bay file.

That's a quick fix ;-) You should be able to do that independent of all other changes, I think?

Final cleanup of all code.

Hmm. The code should be as clean as possible in the first run. Preferably any PR is in a releasable state.

[pgScorpio]

Concerning the proposed changes: I think
....

First redesign of CSoundBase definition so it potentially can accommodate any Sound implementation. (Change platform CSound's and Client code accordingly.

Can you elaborate? What exactly are you planning?

Final cleanup of all code.

Hmm. The code should be as clean as possible in the first run. Preferably any PR is in a releasable state.

Of coarse, but those two are related to the stepwise merging...
In the the first stage CSoundBase will still be compatible with the old code (By implementing backwards compatibility functions in CSoundbase) So it can be merged without changing any non Sound code.
After all new functionality is stepwise implemented in the rest of the code the "cleanup" can be done, which would be removing the backwards compatibility functions from CSoundbase and, possibly, we will than encounter some missed changes in the rest of the code. Finally there probably will be some further improvements possible in CSoundbase and CSound's after the compatibility code is removed.

All Interface related settings should be read and saved by the CSound implementation.

I think that's somewhat a farther away, independent goal? Could potentially clash with the settings redesign by pljones.

Yes, I know, and I would like to discuss several possibilities with @pljones but unfortunately I still didn't get any reaction. Also I don't see any PR/Issue/Discussion on this subject and his "settings-redesign" branch seems to have died some time ago too...
EDIT: Found the pljones branch again,
but it still doesn't seem to have any changes I would expect.

Jack should also implement a Device Setup dialog ('Jack Settings') to, at least, select a Jack patch bay file.

That's a quick fix ;-) You should be able to do that independent of all other changes, I think?

Well I don't think that's a quick fix at all and It will also depend on the sound-redesign implementation and any new implementation of settings handling... And if we do it I want it done the right way right away, so that PR has to wait for quite a while.

[ann0see]

Thanks for adding the checklist!

[ann0see]

@pgScorpio currently some maintainers are on holiday so approving our PRs seems to take some time. In the mean time, I've mentioned you in #533 I think it's worth also thinking about implementing the ability to disconnect from JACK.

[ann0see]

Found another related issue: #1812 (will cross link and close it)

[ann0see]

@pgScorpio I've now force merged my PR. Hopefully this moves the redesign forward.

[pgScorpio]

@ann0see

@pgScorpio I've now force merged my PR. Hopefully this moves the redesign forward.

It"s certainly a step in the right direction, thanks.
But I'm hoping that messages and commandline parameter PR's will be merged too. since my current redesign is based on them too. (These where the result of your request to split into smaller PR's.)