search

kmatheussen / radium

A graphical music editor. A next generation tracker.
http://users.notam02.no/~kjetism/radium/
GNU General Public License v2.0
835 stars 36 forks source link

When current instrument is locked, clicking on an instrument in the mixer should still change current instrument #1412

Open kmatheussen opened 6 months ago

kmatheussen commented 6 months ago
  1. Start Radium
  2. Open instrument widget
  3. Click the "lock" icon in the upper left corner
  4. Click on a different instrument in the mixer
  5. Current instrument is not changed.

Also, after fixing this, the "lock" icon functionality should probably be enabled by default. It's usually annoying that the program changes current instrument for you. Perhaps current instrument should only change if clicking on it in the mixer, or manually changing current track in the editor (i.e. not when it's changing because we play a song and a new current track has a different instrument when playing a new block). Then we don't need the "lock" icon.

yust1n commented 4 months ago

A fix for this will be commited soon. :)

yust1n commented 4 months ago

Done.

kmatheussen commented 4 months ago

Great! Seems to work for the modular mixer. Should be implemented for the mixer strips as well before closing.

kmatheussen commented 4 months ago

And instrument should perhaps change when explicitly changing editor track. Or at least it should be possible to configure the program to do so.

kmatheussen commented 4 months ago

I think this patch should fix the mixer strips. Wonder if it was originally meant to behave like this. The name of the last argument for setCurrentInstrument is a little bit backwards.

kjetil@fedora:/mnt/kjetil/radium$ git diff bin/scheme/mixer-strips.scm
diff --git a/bin/scheme/mixer-strips.scm b/bin/scheme/mixer-strips.scm
index f2d22b518..2672945b1 100644
--- a/bin/scheme/mixer-strips.scm
+++ b/bin/scheme/mixer-strips.scm
@@ -794,7 +794,7 @@
                                                      (<ra> :switch-instrument-is-selected instrument-id))
                                                     ((not (equal? instrument-id (<ra> :get-current-instrument)))
                                                      ;;(c-display "         SETTING CURRENT")
-                                                     (<ra> :set-current-instrument instrument-id #f #t)
+                                                     (<ra> :set-current-instrument instrument-id #f #f)
                                                      ;;(remake-mixer-strips instrument-id)
                                                      )))))
yust1n commented 4 months ago

Your patch seems to work.

kmatheussen commented 4 months ago

This commit seems to ensure current instrument is changed when changing current editor track with the keyboard:

https://github.com/kmatheussen/radium/commit/b520d2d2ff853bdacb99636eec7d9a70d7100bf7

Any other situations now? Hopefully it's possible to remove the "locked instrument" option soon. I don't think anyone wants the non-locked option anymore...

kmatheussen commented 4 months ago

I guess it makes sense for the "lock" icon in the upper left corner to be replaced with a "switch current instrument" icon now...

kmatheussen commented 4 months ago

Problem introduced after last commit: Radium can (wrongly) change instrument automatically when moving the horizontal scrollbar in the editor.

kmatheussen commented 4 months ago

Problem introduced after last commit: Radium can (wrongly) change instrument automatically when moving the horizontal scrollbar in the editor.

Also fixed.

kmatheussen commented 4 months ago

Replaced locked checkbox with select button. TODO: Fix right-clicking the select-button.

kmatheussen commented 4 months ago

TODO: Can't select midi instruments from the select-button.

kmatheussen commented 4 months ago

TODO: Immediately set current instrument to the instrument of the current track in the editor when moving the cursor in the editor, not just when switching between tracks.

kmatheussen commented 4 months ago

TODO: Clean up code, replace all the "even_if..." variables with "also_set_current_instrument" variables, etc.

yust1n commented 4 months ago

In file included from Qt/mQt_patch_widget_callbacks.h:10, from Qt/Qt_instruments.cpp:180: Qt/Qt_patch_widget_callbacks.h: In constructor 'Patch_widget::Patch_widget(QWidget, Patch)': Qt/Qt_patch_widget_callbacks.h:206:5: error: 'locked_instrument' was not declared in this scope; did you mean 'make_instrument'? 206 | locked_instrument->_hovered_callback = [is_alive](bool do_enter){ | ^~~~~ | make_instrument make: *** [Makefile:1661: /tmp/radium_objects_makepkg/Qt_instruments.o] Error 1 Radium compiled. (6s) ==> ERROR: A failure occurred in build(). Aborting... Another TODO :D

kmatheussen commented 4 months ago

Fixed

On Mon, Feb 26, 2024 at 9:53 AM yust1n @.***> wrote:

In file included from Qt/mQt_patch_widget_callbacks.h:10, from Qt/Qt_instruments.cpp:180: Qt/Qt_patch_widget_callbacks.h: In constructor 'Patch_widget::Patch_widget(QWidget, Patch)': Qt/Qt_patch_widget_callbacks.h:206:5: error: 'locked_instrument' was not declared in this scope; did you mean 'make_instrument'? 206 | locked_instrument->_hovered_callback = [is_alive](bool do_enter){ | ^~~~~ | make_instrument make: *** [Makefile:1661: /tmp/radium_objects_makepkg/Qt_instruments.o] Error 1 Radium compiled. (6s) ==> ERROR: A failure occurred in build(). Aborting... Another TODO :D

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1412#issuecomment-1963604886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3JZEWDQD2JJ5KXZ65FDYVRERLAVCNFSM6AAAAABANXS44KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTGYYDIOBYGY . You are receiving this because you authored the thread.Message ID: @.***>

yust1n commented 4 months ago

Outch. Please remove my last commit. I was still testing before committing and didn't expect you to be around.

kmatheussen commented 4 months ago

No it's fine. :-) That code is not going to be used anyway. I just left it there (#if 0-ed out) so that it could serve as a template for what to do when hovering the "select instrument" button, which hasn't been implemented yet.

On Mon, Feb 26, 2024 at 10:59 AM yust1n @.***> wrote:

Outch. Please remove my last commit. I was still testing before committing and didn't expect you to be around.

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1412#issuecomment-1963730118, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3J2GXJQ7KW6S2V7WH53YVRMHJAVCNFSM6AAAAABANXS44KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRTG4ZTAMJRHA . You are receiving this because you authored the thread.Message ID: @.***>

yust1n commented 4 months ago

MIDIinstruments also don't get displayed in the modularmixer :(

kmatheussen commented 4 months ago

TODO: Remove all "force as current instrument" menu options.

kmatheussen commented 4 months ago

MIDIinstruments also don't get displayed in the modularmixer :(

Yeah, a MIDI instrument is a different type of thing. It doesn't make sense adding MIDI instruments in its current form into the modular mixer. What should be done instead is to write new "Audio instrument"s that handle MIDI. Afterwards (in the code) remove "MIDI instrument", and rename "Audio instrument" to just "Instrument".

kmatheussen commented 4 months ago

TODO: Update manual.

kmatheussen commented 4 months ago

Update: all "TODO"s are done, except this one:

"TODO: Clean up code, replace all the "even_if..." variables with "also_set_current_instrument" variables, etc.".

But that's just an internal thing. It should be ready for release now...

kmatheussen commented 4 months ago

TODO: Check that the program does the right things (whatever that might be) when deleting and creating instruments.

yust1n commented 4 months ago

Hmmm ... maybe selecting a track in the editor via mouse should also change the instrument ? Guess I need to learn some scheme.

kmatheussen commented 4 months ago

Yes, adding it now. TODO: Selecting a track in the editor via mouse should probably change current instrument.

kmatheussen commented 4 months ago

Guess I need to learn some scheme.

The big advantage of writing code in scheme in Radium is that you don't have to compile anything when doing changes. You don't even have to restart Radium.

For instance, when fixing a bug in a function, first I edit the text in emacs, and then - while the cursor is still placed inside the function I edited - I press Ctrl+Alt+x. That's it.

To make emacs behave like, it's only necessary to add this line to ~/.emacs: (setq scheme-program-name "/path/to/radium/bin/s7webserver/s7webserver_repl.py")

yust1n commented 4 months ago

I use vim .) So in other words it just saves the edited scm.file and executes webserver_repl.py ?

kmatheussen commented 4 months ago

I use vim .) So in other words it just saves the edited scm.file and executes webserver_repl.py ?

No, webserver_repl.py is a REPL. It's a program, running all the time, that reads from stdin and replies to stdout. Ctrl+Alt+x only sends the block of code the cursor is currently placed in to the REPL. If you want to evaluate the whole file instead, the shortcut to use is ctrl+c and afterwards press l. I believe this is the most popular way to do programming in Lisp. I'm sure you can do this in VIM too.

kmatheussen commented 4 months ago

At one point in my twentieth I tried to switch to VIM, but I gave up after about a month. I had been too accustomed to emacs so it wasn't possible to make the switch. :-)

kmatheussen commented 4 months ago

Update: All TODOs, except the internal one, are finished. May be ready for release again.

yust1n commented 4 months ago

Found an issue when asigning an existing instrument to a track in the editor. Cursor needs to be moved to show the selected instrument. I also noticed that an existing instrumentname was renamed by a new sampleplayer. The new instrument also was just named sampleplayer and still doesn't care when I change the loaded sample. When clicking the trackname in the editor the instrument doesn't change.

yust1n commented 4 months ago

https://iainctduncan.github.io/learn-scheme-for-max/part_1.html This seems a nice place to start with scheme/lisp.

kmatheussen commented 4 months ago

Thanks, adding:

TODO:

  1. Switch current instrument when assigning an existing instrument to a track in the editor.
  2. Editor track header doesn't update when changing current instrument for track. (Recipe: 1: Assign a new instrument to a track. 2. Assign an existing instrument to the same track. 3. Instrument is assigned, but the header is not uptodate.)
  3. Current instrument should probably change when clicking trackname in the editor. I purposedly didn't implement this, but I see now that it should probably change current instrument

https://iainctduncan.github.io/learn-scheme-for-max/part_1.html Yes, very well-written.

kmatheussen commented 4 months ago

TODO: Add recipe to the manual on how to develop in scheme in Radium using emacs. (What to add to ~/.emacs, how to start the repl (I.e. M-x run-scheme), how to evaluate code)

yust1n commented 3 months ago

I start disliking this new feature. It's pretty annoying while editing when the instrument stays on an instrument of the block before that doesn't even exist in the actual block. Maybe it's better to much this stuff to an experimental branch until it works proper and doesn't interrupt the workflow like it does at the moment.

kmatheussen commented 3 months ago

It's pretty annoying while editing when the instrument stays on an instrument of the block before that doesn't even exist in the actual block.

Sorry, I don't understand this. Are you talking about mouse or keyboard?

Maybe it's better to much this stuff to an experimental branch until it works proper and doesn't interrupt the workflow like it does at the moment.

Ouch, sure we can't fix the annoying parts?

kmatheussen commented 3 months ago

I actually thought the old behavior was annoying, so I wanted to fix that. But if this is more annoying, then obviously it shouldn't be included in the next release.

kmatheussen commented 3 months ago

To clarify, what I found very annoying before was that the current instrument was changed when playing a song, so that the current instrument always was the same as the current track in the currently playing block in the current seqtrack.

Everything else should perhaps be as before, as you suggest now, but I also want to get rid of the "lock instrument" button in the instrument widget, and replace it with a "select instrument" button, as has been done.

I hope we don't have to revert the code. I don't think it is that much work to just manually fix the annoying parts.

yust1n commented 3 months ago

So only lock the instrument while playing a song ? Maybe also keep the lockinstrumentbutton and make it an instrumentchooser by rightclicking it ? This way the transition would hurt less ;D

kmatheussen commented 3 months ago

Excellent. Thanks for raising this before release.

yust1n commented 3 months ago

I found other stuff that should also be fixed like the trackmuter in the sequencer. It changes when you are hovering over another block. Unfortunatelly I had more than one sequencertrack and I had to move that window close to the block I wanted to set the mutes. https://github.com/kmatheussen/radium/issues/1049#issuecomment-1986880404 These options are hidden pretty well. I would love to be able to have an easier access. "Play notes under the cursor when moving up or down" is also hard to find/access. I bet this is all hidden somewhere in scheme. So take this as a reminder for me where to start searching :) Enable Multisamplesupport like .xi or .sf2 in sampleplayer. .sf2 Works in Drums but I want to see waveforms :) and that's an essential thing for a tracker, isn't it ? Also check keyspan in sampleplayer.

kmatheussen commented 3 months ago

.sf2 Works in Drums but I want to see waveforms :)

A quick fix for that is to use the sample player instrument to load the drums.sf2 file (yes, the sample player instrument supports sf2!). Fluidsynth does some magic that often makes sf2 files sound much better though (probably a different type of resampler algorithm), but maybe it doesn't sound that much better for drums.

kmatheussen commented 3 months ago

Hmm, seems to be bugs in the sample mapping of the sample player instrument when using drums.sf2.

yust1n commented 3 months ago

Whenever I tried it there was only one sample being played. <<< FluidR3_GM_drums.sf2 also works Same with xiinstruments. <<< wrong !!! Just checked it and it works !!! OMG. I rolled back to https://github.com/kmatheussen/radium/commit/822cb9ab3d1b5205351e54bdcd06c07979a74ccd and just discovered that you already had the instrumentchooser hidden in a submenu on rightclick.

yust1n commented 3 months ago

Found the source of them problem by checking the soundfonts in Polyphone. When a soundfont uses multiple instruments for a preset it won't work proper. Same sf2-files has no problems in fluidsynth.

kmatheussen commented 3 months ago

Yes, I see that. But I'm pretty sure it was supposed to work properly. :-)

On Mon, Mar 11, 2024 at 1:40 PM yust1n @.***> wrote:

Found the source of them problem by checking the soundfonts in Polyphone. When a soundfont uses multiple instruments for a preset it won't work proper. Same sf2-files has no problems in fluidsynth.

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1412#issuecomment-1988349109, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3J6KYL7PA6TQYPT3XQLYXWQ2ZAVCNFSM6AAAAABANXS44KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGM2DSMJQHE . You are receiving this because you authored the thread.Message ID: @.***>

kmatheussen commented 3 months ago

I can't test Radium right now, but maybe the block below this comment in the bottom Sampler_plugin_sf2_load.c is causing this:

"// Optimize data->notes so that as few Note objects as possible are used (better for cache)"

I see that this block was added many years after the initial code was written. Perhaps I didn't test sf2 after adding this code.

On Mon, Mar 11, 2024 at 1:41 PM Kjetil Matheussen @.***> wrote:

Yes, I see that. But I'm pretty sure it was supposed to work properly. :-)

On Mon, Mar 11, 2024 at 1:40 PM yust1n @.***> wrote:

Found the source of them problem by checking the soundfonts in Polyphone. When a soundfont uses multiple instruments for a preset it won't work proper. Same sf2-files has no problems in fluidsynth.

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1412#issuecomment-1988349109, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3J6KYL7PA6TQYPT3XQLYXWQ2ZAVCNFSM6AAAAABANXS44KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGM2DSMJQHE . You are receiving this because you authored the thread.Message ID: @.***>

kmatheussen commented 3 months ago

Or maybe it wasn't implemented. I see this comment in the code:

" // Try to find an instrument from the region. A preset may use several instruments, but that's not supported yet. We just use the first and best/worst."

Multiple samples should work, but it seems like there is a higher concept of "instruments" in sf2 as well.

On Mon, Mar 11, 2024 at 1:52 PM Kjetil Matheussen @.***> wrote:

I can't test Radium right now, but maybe the block below this comment in the bottom Sampler_plugin_sf2_load.c is causing this:

"// Optimize data->notes so that as few Note objects as possible are used (better for cache)"

I see that this block was added many years after the initial code was written. Perhaps I didn't test sf2 after adding this code.

On Mon, Mar 11, 2024 at 1:41 PM Kjetil Matheussen < @.***> wrote:

Yes, I see that. But I'm pretty sure it was supposed to work properly. :-)

On Mon, Mar 11, 2024 at 1:40 PM yust1n @.***> wrote:

Found the source of them problem by checking the soundfonts in Polyphone. When a soundfont uses multiple instruments for a preset it won't work proper. Same sf2-files has no problems in fluidsynth.

— Reply to this email directly, view it on GitHub https://github.com/kmatheussen/radium/issues/1412#issuecomment-1988349109, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAIX3J6KYL7PA6TQYPT3XQLYXWQ2ZAVCNFSM6AAAAABANXS44KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBYGM2DSMJQHE . You are receiving this because you authored the thread.Message ID: @.***>

yust1n commented 3 months ago

There is also a problem with stereosamples in a soundfont. Pitch is too low. Atleast the slicedbeats I created with shuriken import now fine in Radium when I convert them into mono and use only one instrument.