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.
+ */
+bool
+midi_region_is_note_playable (
+ const ZRegion * self,
+ const MidiNote * midi_note
+)
+{
+ 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
+ * @param repeat_index repetition counter for loop offset
+ */
+void
+midi_region_set_note_positions_in_export (
+ 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;
+}
+
/**
* 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;
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 さんは書きました:
> 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 */
s/minimum/maximum/
> +#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 (
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.
> + 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);
Please send an updated patch and I will merge it after testing it works
as intended and doesn't break the current export.
--
Alex