Re: [LMMS-devel] PATCH: Recording support

[Javier S. P. <ja...@te...>]

En/na Jonas ha escrit:
> Hi everyone,
> 
> I have made a patch to LMMS that adds more flexible recording support.

Hi, Jonas.

I've tested your patch. Unfortunately, it isn't ready to be included IMHO.

> The changes consist of two main parts.
> 
> 1. I have added code to 'songEditor::processNextBuffer' to map the
> current play position to 'piano_roll' units. This makes the play
> position indicator of the piano roll move when the song or BB is being
> played.

Got the idea. It needs some polish. But after using the play_song mode,
I get a segfault when loading a pattern.

> 2. I had to make additional changes to 'pianoRoll::recordNote' to get
> the start position of the recorded note, in face of looping and multiple
> BB items. This duplicates some of the logic introduced in 1 - but since
> the recording gets the note only at note end this seems hard to avoid.
> 
> I have not made any changes in the user interface. To use the new
> recording functionallity, press play in the song or BB editor, and then
> turn on recording by pressing record (in the piano roll). This was
> implemented for testing, a better interface could definately be introduced.

I'm quite annoyed with that feature: recording while looping? Is that
useful? I expected the normal recording behaviour.

> I do not quite understand how the threading of the application works,
> and if I need to protect some access with a mutex - the code seems
> stable though...
> 
> As a side note, the patch also extends the virtual keyboard with a few
> keys. (This should work regardless of keyboard layout)

I like the upper keys extension. About the lower ones, there shouldn't
be any repeated key. Pressing the same note with two different keys
(yes, I do that) ends up in no sound.
We could shift the lower keyboard to the right and gain seven additional
notes. But I'd like to hear some feedback from (non-MIDI) keyboard users
before.

> Take care
> /Jonas

We really appreciate your effort. Needless to say, this is only my
viewpoint. Other developers here may consider to include your patch
right now.
IMHO you've picked a bit complex part to start with. Another complex but
safer choice is to implement the slider we've been mentioning recently
(ask for guidance first). Of course, you can work further on your patch
or wait until someone fixes it.

Bye.