Record n beats - Githubissues

georgkrause commented 6 years ago

since there seems to be no movement on #229 I rebased the code to the current master and fixed the issue with the mouse interaction. Should be ready to merge. After this we can do a feature freeze and do the release-testing :)

Edit: After some discussions here, there is more work to be done:

[x] Count in bars, not beats
[ ] Update length earlier to avoid confusion
[x] get all values from DSP-instance, do not store them anywhere else!
[ ] add non member functions to (anonymous) namespace
[ ] isolate actual and to be recorded length in event structure
[ ] reset beats to record when loading a clip
[ ] add global default length, which can be overwritten per clip

vale981 commented 6 years ago

I didn't like to do it that way either, but I had a reason. I can't remember it right now, because i am out of touch with the matter.

I suppose it had something to do with me not wanting to use the messaging protocols every time I wanted to get that value.

In hindsight that might just be the proper way however.

On 5 July 2018 00:45:52 CEST, Harry van Haaren notifications@github.com wrote:

harryhaaren commented on this pull request.

@@ -90,6 +91,34 @@ void ClipSelector::clipName(int clip, std::string name) redraw(); }

+void ClipSelector::setClipBeats(int scene, int beats, bool isBeatsToRecord) +{

clips[scene].setBeats(beats, isBeatsToRecord);

redraw(); +}

+double getCairoTextWith(cairo_t cr, const char str)

just make them "static" and that way the C++ compiler won't make them visible from outside the compilation unit, aka .cpp file

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#discussion_r200207725

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

vale981 commented 6 years ago

Sorry. The previous comment was supposed to be about the setClipBeats matter. I am replying on my mobile via email. Things got mixed up.

On 5 July 2018 00:46:31 CEST, Harry van Haaren notifications@github.com wrote:

harryhaaren commented on this pull request.

@@ -90,6 +91,34 @@ void ClipSelector::clipName(int clip, std::string name) redraw(); }

+void ClipSelector::setClipBeats(int scene, int beats, bool isBeatsToRecord) +{

clips[scene].setBeats(beats, isBeatsToRecord);

agreed, the DSP instance should have all the "official" values, and the GUIs/controllers only a view of that

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#discussion_r200207762

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

vale981 commented 6 years ago

This is called by an event if I am not mistaken.

On 5 July 2018 10:30:24 CEST, Georg Krause notifications@github.com wrote:

georgkrause commented on this pull request.

@@ -90,6 +91,34 @@ void ClipSelector::clipName(int clip, std::string name) redraw(); }

+void ClipSelector::setClipBeats(int scene, int beats, bool isBeatsToRecord) +{

clips[scene].setBeats(beats, isBeatsToRecord);

@harryhaaren so we need a new event for this?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#discussion_r200270450

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

vale981 commented 6 years ago

The negative numbers have a special meaning.

On 5 July 2018 10:37:09 CEST, Georg Krause notifications@github.com wrote:

georgkrause commented on this pull request.

RECORD_LENGTH_MENU_ITEM(1),

RECORD_LENGTH_MENU_ITEM(2),

RECORD_LENGTH_MENU_ITEM(4),

RECORD_LENGTH_MENU_ITEM(8),

RECORD_LENGTH_MENU_ITEM(16),

RECORD_LENGTH_MENU_ITEM(32),

RECORD_LENGTH_MENU_ITEM(64),

{0},

{ "Bars to record", 0, 0, 0, FL_SUBMENU | FL_MENU_DIVIDER},

RECORD_BARS_MENU_ITEM(1),

RECORD_BARS_MENU_ITEM(2),

RECORD_BARS_MENU_ITEM(4),

RECORD_BARS_MENU_ITEM(6),

RECORD_BARS_MENU_ITEM(8),

{"Endless", 0, setRecordBarsCb, (void*)-1, FL_MENU_DIVIDER | FL_MENU_RADIO | ((clips[clipNum].getBeatsToRecord() <= 0) ? FL_MENU_VALUE : 0) | (empty ? 0 : FL_MENU_INACTIVE)},

{"Custom", 0, setRecordBarsCb, (void*)-2, empty ? 0 : FL_MENU_INACTIVE},

@harryhaaren Do you think passing -2 as content is a problem here? Seems to be the same like in the other menu entries...

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#pullrequestreview-134553809

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

georgkrause commented 6 years ago

@vale981 I know they have a special meaning, but @harryhaaren wrote me this seems to be a little hacky and I try to understand what might be the problem here ;)

vale981 commented 6 years ago

From my perspective, the hacky thing is the cast to (void*) -- Sent from my Android device with K-9 Mail. Please excuse my brevity.

coderkun commented 6 years ago

This feature is great and helps me a lot to record loops properly with two hands on the keys for instance. However it is not very convenient to righ-click on each clip to select the length for each loop individually. To use Luppp for live performances it is best to keep the mouse interactions to a minimum.

My suggestion for this feature is therefore to not have the length setting per clip but either have a global setting which controls how long the next recorded loop will be or one per track. The length could be displayed as a big number somewhere or—in case of the track setting—below the track title. Together with a MIDI binding to change the length (e. g. with some kind of wheel controller) this would be a really neat feature.

georgkrause commented 6 years ago

@coderkun we already discusses a global default value for the clip length here: https://github.com/openAVproductions/openAV-Luppp/pull/201#issuecomment-377890198 (and following comments)

So we have different alternatives here...

global default (which can be changed in settings or controller/per clip)
clip length setting per track

EDIT 1' later: I am not sure if we should include this here or focus on finishing this first and change something on it later.

georgkrause commented 6 years ago

After some discussions on the Riot channel I would like to add some thoughts here. It seems like a nice workflow to set the default length of the clip to be recorded per track and add an option to (de)activate the feature per track. So you can make track one record 4 bars if the button is enabled. This need some GUI change, so we need to find a nice place to display the length setting and add a button to enable/disable the feature. But this offers some nice possibilities but is easy to control. Anyway, adding more elements to the GUI can result in an overflow for the user and should be really well considered.

vale981 commented 6 years ago

Maybe we can make that feature optional all together, so that you enable / disable this extra button in the config.

On 22 July 2018 20:40:31 CEST, Georg Krause notifications@github.com wrote:

After some discussions on the Riot channel I would like to add some thoughts here. It seems like a nice workflow to set the default length of the clip to be recorded per track and add an option to (de)activate the feature per track. So you can make track one record 4 bars if the button is enabled. This need some GUI change, so we need to find a nice place to display the length setting and add a button to enable/disable the feature. But this offers some nice possibilities but is easy to control. Anyway, adding more elements to the GUI can result in an overflow for the user and should be really well considered.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#issuecomment-406887596

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

vale981 commented 6 years ago

That's how kde does it. They add a ton of features, which you can disable to keep it minimal.

Anyhow. That feature which we are discussing is so esential, that I doubt anyone will be disabling it. Actually there is a nice fork of giada, which has multiple record modes (like overdub, overwrite etc.). Something like that wouldn't be too bad either.

On 22 July 2018 20:40:31 CEST, Georg Krause notifications@github.com wrote:

After some discussions on the Riot channel I would like to add some thoughts here. It seems like a nice workflow to set the default length of the clip to be recorded per track and add an option to (de)activate the feature per track. So you can make track one record 4 bars if the button is enabled. This need some GUI change, so we need to find a nice place to display the length setting and add a button to enable/disable the feature. But this offers some nice possibilities but is easy to control. Anyway, adding more elements to the GUI can result in an overflow for the user and should be really well considered.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/openAVproductions/openAV-Luppp/pull/239#issuecomment-406887596

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

georgkrause commented 6 years ago

About deactivating this:

Its only possible via config file at the moment, so it should be activated by default and only power users can disable it. Dont know if that makes sense at all. There is #221.
The GUI is currently kind of static, so adding/removing stuff on time time gets more and more difficult.

In general, if we want to save space and this is also usable and flexible enough, @coderkun proposed to have a global button to enable/disable the record n beats mode. From my point of view its kind of important to have the setting for the length per track to enable the user to say: I want to have drum loops on Track 1 with a length of 1 bar and a bass on track 2 with 4 bars length. But if you want free recording, you simply disable finite recording.

georgkrause commented 6 years ago

@vale981 if you have ideas for more recording/playback modes, please create different tickets. In general I would love to see something going on in this area, but as always this needs some developer time. You have some to spend? :)

georgkrause commented 6 years ago

I updated the first post and added a list of todos. Please review and ping me to add something if necessary!

georgkrause commented 6 years ago

@harryhaaren @vale981 I had a deeper look at the problematic of different locations to store the length of the clip. Actually it was wrong to criticize this one. Other values are stored in the ClipState-Objects, too, also there is maybe no other possibility since you cannot poll the DSP-Instances. At least, I don't have any idea. ;) The last commit improves this a little, so from now the length is not stored three times in clip state but one time and converted in the needed values.

openAVproductions / openAV-Luppp

Record n beats #239