Robert Panovics: 1 Draft implementation of base/full midi export 2 files changed, 171 insertions(+), 12 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~alextee/zrythm-devel/patches/30734/mbox | git am -3Learn more about email & git
Signed-off-by: Robert Panovics <robert.panovics@gmail.com> --- inc/audio/position.h | 8 ++ src/audio/midi_region.c | 175 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 171 insertions(+), 12 deletions(-) diff --git a/inc/audio/position.h b/inc/audio/position.h index 2a23ccf3e..987c95593 100644 --- a/inc/audio/position.h +++ b/inc/audio/position.h @@ -125,6 +125,14 @@ (position_is_after (_pos, _start) && \ position_is_before (_pos, _end)) +/** Returns minimum of p1 and p2 */ +#define position_min(p1,p2) \ + (position_compare(p1,p2) < 0 ? p1 : p2) + +/** Returns minimum of p1 and p2 */ +#define position_max(p1,p2) \ + (position_compare(p1,p2) > 0 ? p1 : p2) + /** Inits the default position on the stack. */ #define POSITION_INIT_ON_STACK(name) \ Position name = POSITION_START; diff --git a/src/audio/midi_region.c b/src/audio/midi_region.c index 1e58e3961..a884c2b10 100644 --- a/src/audio/midi_region.c +++ b/src/audio/midi_region.c @@ -36,6 +36,7 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. */ +#include <tgmath.h> #include "audio/channel.h" #include "audio/exporter.h" @@ -959,6 +960,115 @@ midi_region_get_midi_ch ( return ret; } +/** + * Checks if note is playable.
Please change the comment to: Returns whether the given note is not muted and starts within any playable part of the region.
+ */ +bool +midi_region_is_note_playable ( + const ZRegion * self, + const MidiNote * midi_note +)
This should be declared in the header.
+{ + ArrangerObject * self_obj = + (ArrangerObject *) self; + + ArrangerObject * mn_obj = + (ArrangerObject *) midi_note; + + if (arranger_object_get_muted (mn_obj, false)) + { + return false; + } + + if (!position_is_between (&mn_obj->pos, + &self_obj->loop_start_pos, &self_obj->loop_end_pos) && + !position_is_between (&mn_obj->pos, + &self_obj->clip_start_pos, &self_obj->loop_start_pos)) + { + return false; + } + + return true; +} + +/** + * Set positions to the exact values in the export + * region as it is played inside Zrythm. + * + * @param start_pos start position of the event + * @param end_pos end position of the event
Please make start_pos and end_pos be `@param[in,out]` instead of just `@param` to be clearer.
+ * @param repeat_index repetition counter for loop offset + */ +void +midi_region_set_note_positions_in_export (
Should be static void, and you can drop the `midi_region_` prefix. Should be renamed to `get_note_positions_in_export()` since it's returning the positions.
+ const ZRegion * self, + Position * start_pos, + Position * end_pos, + int repeat_index +) +{ + ArrangerObject * self_obj = + (ArrangerObject *) self; + + double loop_length_in_ticks = + arranger_object_get_loop_length_in_ticks (self_obj); + POSITION_INIT_ON_STACK (export_start_pos); + POSITION_INIT_ON_STACK (export_end_pos); + position_add_ticks (&export_end_pos, + arranger_object_get_length_in_ticks (self_obj)); + + position_set_to_pos (end_pos, + position_min (&self_obj->loop_end_pos, + end_pos)); + + if (position_is_before (start_pos, + &self_obj->clip_start_pos)) + { + ++repeat_index; + } + + position_add_ticks (start_pos, + loop_length_in_ticks * repeat_index - + position_to_ticks (&self_obj->clip_start_pos)); + position_add_ticks (end_pos, + loop_length_in_ticks * repeat_index - + position_to_ticks (&self_obj->clip_start_pos)); + position_set_to_pos (start_pos, + position_max (start_pos, &export_start_pos)); + position_set_to_pos (end_pos, + position_min (end_pos, &export_end_pos)); +} + +/** + * Returns if the given positions are in a given + * region as it is played inside Zrythm. + * + * @param offset_in_ticks Offset value if note is + * repeated inside a loop + */ +bool +midi_region_is_note_in_full_region ( + const ZRegion * self, + const Position * start_pos +) +{ + ArrangerObject * self_obj = + (ArrangerObject *) self; + + POSITION_INIT_ON_STACK (export_start_pos); + POSITION_INIT_ON_STACK (export_end_pos); + position_add_ticks (&export_end_pos, + arranger_object_get_length_in_ticks (self_obj)); + + if (!position_is_between (start_pos, + &export_start_pos, &export_end_pos)) + { + return false; + } + + return true;
Can just simplify this to:
return
position_is_between (
start_pos, &export_start_pos, &export_end_pos);
+} + /** * Adds the contents of the region converted into * events. @@ -984,6 +1094,14 @@ midi_region_add_events ( if (add_region_start) region_start = self_obj->pos.ticks; + double loop_length_in_ticks = + arranger_object_get_loop_length_in_ticks (self_obj); + int number_of_loop_repeats = + (int)ceil((arranger_object_get_length_in_ticks (self_obj) - + position_to_ticks (&self_obj->loop_start_pos) + + position_to_ticks (&self_obj->clip_start_pos)) / + loop_length_in_ticks); + MidiNote * mn;
This is from my previous code but it should probably go inside the loop.
for (int i = 0; i < self->num_midi_notes; i++) { @@ -991,18 +1109,51 @@ midi_region_add_events ( ArrangerObject * mn_obj = (ArrangerObject *) mn; - midi_events_add_note_on ( - events, 1, mn->val, mn->vel->vel, - (midi_time_t) - (position_to_ticks (&mn_obj->pos) + - region_start), - F_NOT_QUEUED); - midi_events_add_note_off ( - events, 1, mn->val, - (midi_time_t) - (position_to_ticks (&mn_obj->end_pos) + - region_start), - F_NOT_QUEUED); + if (full && !midi_region_is_note_playable (self, mn)) + { + continue; + } + + int repeat_counter = 0; + bool write_only_once = true; + + do + { + Position mn_pos = mn_obj->pos; + Position mn_end_pos = mn_obj->end_pos; + + if (full) + { + if (position_is_between(&mn_obj->pos, + &self_obj->loop_start_pos, &self_obj->loop_end_pos)) + { + write_only_once = false; + } + + midi_region_set_note_positions_in_export( + self, &mn_pos, &mn_end_pos, repeat_counter); + + if (!midi_region_is_note_in_full_region ( + self, &mn_pos)) + { + continue; + } + } + + midi_events_add_note_on ( + events, 1, mn->val, mn->vel->vel, + (midi_time_t) + (position_to_ticks (&mn_pos) + + region_start), + F_NOT_QUEUED); + midi_events_add_note_off ( + events, 1, mn->val, + (midi_time_t) + (position_to_ticks (&mn_end_pos) + + region_start), + F_NOT_QUEUED); + } while (++repeat_counter < number_of_loop_repeats && + !write_only_once); } midi_events_sort (events, F_NOT_QUEUED); -- 2.35.1
Thanks. On first glance it seems ok but I haven't tested it. My general opinion is that it needs a test that tries to export a MIDI region in both export types, but I can write it later myself if you don't want to do it. Also if you're making significant changes (more than 10~15 lines of actual logic) to a file like in this case please add your copyright notice at the top, e.g. Copyright (C) 2022 Robert Panovics <robert.panovics at gmail dot com> Also please build with `-Dstrict_flags=true`, it should throw some errors here because you're using non-static functions without declarations. 2022-04-01 (金) の 23:26 +0200 に Robert Panovics さんは書きました:
s/minimum/maximum/
Either make static and drop the `midi_region_` prefix or add a declaration in the header. Also since you're not actually passing a note but the start position of a note in the export it should be named `is_note_export_start_pos_in_full_region()` or something similar to be more descriptive.
Please send an updated patch and I will merge it after testing it works as intended and doesn't break the current export. -- Alex