jmamma / MCL

MCL firmware for the MegaCommand MIDI Controller.
BSD 2-Clause "Simplified" License
53 stars 9 forks source link

ExtSeqTrack fixes required before 3.0 #140

Closed jmamma closed 3 years ago

jmamma commented 3 years ago
533 void ExtSeqTrack::seq() {
534   if (mute_until_start) {
535 
536     if ((clock_diff(MidiClock.div16th_counter, start_step) == 0)) {
537       if (start_step_offset > 0) {
538         start_step_offset--;
539       } else {
540         reset();
541       }
542     }
543   }
544   uint8_t timing_mid = get_timing_mid_inline();
545   if ((MidiUart2.uart_block == 0) && (mute_until_start == false) &&
546       (mute_state == SEQ_MUTE_OFF)) {
547 
548     // the range we're interested in:
549     // [current timing bucket, micro >= timing_mid ... next timing bucket, micro
550     // < timing_mid]
551 
552     uint16_t ev_idx, ev_end;
553 
554     // Locate CURRENT
555     locate(step_count, ev_idx, ev_end);  **<---- OPTIMIZE HIM**

We can keep track of current ev_idx. And then increase/decrease as items are added removed. Use ISR block for consistency.

 68   bool operator<(const ext_event_t &that) {
 69     // order by micro_timing
 70     if (this->micro_timing != that.micro_timing) {
 71       return this->micro_timing < that.micro_timing;
 72     }
 73     /*
 74     // order by lock
 75 
 76     if (this->is_lock != that.is_lock) {
 77       return this.is_lock;
 78     }
 79     */
 80     // off < on
 81     if (this->event_on != that.event_on) {
 82       return that.event_on;
 83     }
 84 
 85     return false;
 86   }
 87 };

Implement order by lock

jmamma commented 3 years ago

Shift left and shift right are broken. Have been before since I was working on piano_roll.

jmamma commented 3 years ago

velocity shifting was missing.

still seems broken.

just to confirm @yatli we are shifting around visible length. not the entire data structure?

jmamma commented 3 years ago

520ee3f

jmamma commented 3 years ago

reverse has mysteriously disappeared from menu options... SEQ_MENU_REVERSE is not disabled anywhere...

jmamma commented 3 years ago

If I add

@@ -85,6 +85,8 @@ void SeqPage::init() {
   seq_menu_page.menu.enable_entry(SEQ_MENU_PIANOROLL, false);
   seq_menu_page.menu.enable_entry(SEQ_MENU_PARAMSELECT, false);
   seq_menu_page.menu.enable_entry(SEQ_MENU_SLIDE, false);
+  seq_menu_page.menu.enable_entry(SEQ_MENU_REVERSE, true);
+

I get a corrupt menu, and then lockup.

@yatli can you investigate what's happening here.

yatli commented 3 years ago

just to confirm @yatli we are shifting around visible length. not the entire data structure?

IIRC yes

jmamma commented 3 years ago

 33 class MenuBase {
 34 public:
 35   uint8_t entry_mask[2];
jmamma commented 3 years ago

we've gone beyond 16bit.

lol.

yatli commented 3 years ago

uint8_t entry_mask[2];

yatli commented 3 years ago

ha you beat me to it

jmamma commented 3 years ago

yes

jmamma commented 3 years ago

it's a 32bit console era now

yatli commented 3 years ago

hmm, how about splitting the seq page menu?

yatli commented 3 years ago

let me prototype it (don't have MC at hand, but anyway I can try it)

jmamma commented 3 years ago

I think we would be doubling up on menu items if we did that.

jmamma commented 3 years ago

Running out of time for 3.0 release. I'm trying to wrap this one up ASAP.

jmamma commented 3 years ago
 985   case DIR_REVERSE:
 986     uint16_t end = ev_end / 2;
 987     for (uint16_t i = 0; i < end; ++i) {
 988       auto tmp = events[i];
 989       auto j = ev_end - i - 1;
 990       events[i] = events[j];
 991       events[j] = tmp;
 992 
 993       // need to flip note on/off
 994       if (!events[i].is_lock) {
 995         events[i].event_on = !events[i].event_on;
 996       }
 997 
 998       if (!events[j].is_lock) {
 999         events[j].event_on = !events[j].event_on;
1000       }
1001     }
1002     for (uint8_t n = 0; n < length; n++) {
1003       uint8_t vel_tmp = velocities[n];
1004       uint8_t z = length - 1 - n;
1005       velocities[n] = velocities[z];
1006       velocities[z] = vel_tmp;
1007     }
1008     timing_buckets.reverse(length);
1009     break;

what's wrong here ^

where

 955   locate(length, ev_idx, ev_end);
yatli commented 3 years ago

Running out of time for 3.0 release. I'm trying to wrap this one up ASAP.

Ok, discarding.

yatli commented 3 years ago

velocity is 0..127 event is 0..511

jmamma commented 3 years ago

yes velocity has been separated. see above ^

yatli commented 3 years ago

yep, but velocity reverse uses length which is for events.

jmamma commented 3 years ago

I think the problem might be to do with microtiming ?

If I have the notes at full step length it's okay ?

yatli commented 3 years ago

microtiming not reversed!

yatli commented 3 years ago

try this:

  case DIR_REVERSE:
    uint16_t end = ev_end / 2;
    uint8_t timing_mid_2x = get_timing_mid() * 2;
    for (uint16_t i = 0; i < end; ++i) {
      auto tmp = events[i];
      auto j = ev_end - 1 - i;
      events[i] = events[j];
      events[j] = tmp;

      // need to flip note on/off
      if (!events[i].is_lock) { events[i].event_on = !events[i].event_on; }
      if (!events[j].is_lock) { events[j].event_on = !events[j].event_on; }

      // reverse microtiming
      events[i].micro_timing = timing_mid_2x - events[i].micro_timing;
      events[j].micro_timing = timing_mid_2x - events[j].micro_timing;
    }
    for (uint8_t n = 0; n < length; n++) {
      uint8_t vel_tmp = velocities[n];
      uint8_t z = length - 1 - n;
      velocities[n] = velocities[z];
      velocities[z] = vel_tmp;
    }
    timing_buckets.reverse(length);
    break;
yatli commented 3 years ago

emm,

    for (uint8_t n = 0; n < length; n++) {
      uint8_t vel_tmp = velocities[n];
      uint8_t z = length - 1 - n;
      velocities[n] = velocities[z];
      velocities[z] = vel_tmp;
    }

you end up swapping twice

jmamma commented 3 years ago

It looks better on the editor, but no audible note playback on reverse.

Also now my shift menu has disappeared

jmamma commented 3 years ago

oh should be length/2

yatli commented 3 years ago

git pull it

jmamma commented 3 years ago

git pull it

?

yatli commented 3 years ago

I pushed one commit for that.

jmamma commented 3 years ago

which branch?

jmamma commented 3 years ago

i'm working in dev

yatli commented 3 years ago

emm, dev?

jmamma commented 3 years ago

commit id ?

yatli commented 3 years ago

0931bc78d249b1eee9f3bba07663f1ddd0f12dcf

jmamma commented 3 years ago

I merged your commit with mine.

Reverse playback is still broken. some notes not playing.

jmamma commented 3 years ago

And shift menu has disappeared since increase entry_mask to 32bit.

yatli commented 3 years ago

05e246da

yatli commented 3 years ago

Reverse playback is still broken. some notes not playing.

What if without microtiming reverse? Just no velocity?

jmamma commented 3 years ago

yep. midi monitor showing 0 vel

jmamma commented 3 years ago

vel_tmp is wrong.

jmamma commented 3 years ago

i've well and truly exhausted my capabality this weekend. day job in 8 hours.

jmamma commented 3 years ago

if you can get the A4 conversion problem sorted that would be a big help. that way I can tick off project conversion.

jmamma commented 3 years ago

i'm off. 🛌

yatli commented 3 years ago

if you can get the A4 conversion problem sorted that would be a big help. that way I can tick off project conversion.

roger roger.

good night!