ModusCreateOrg / trackerEditor

Create 8-bit tracker music for ATMLib2
http://labs.moduscreate.com/trackerEditor/
MIT License
0 stars 3 forks source link

Arpeggiator seems to not function on device #26

Open jaygarcia opened 6 years ago

jaygarcia commented 6 years ago

Arpeggiator seems not function in the current Master. It plays once through and it's done.

Seems to work in the UI. Can't tell where the problem is.

(updated to attach files)

jaygarcia commented 6 years ago

arpeggiator.atm.txt

jaygarcia commented 6 years ago

arpeggiator.h.txt

dxxb commented 6 years ago

New docs:

P2
    Size   : 1 byte
    Name   : Effect configuration
    Format : b-edttttt
              |||└└└└└-> ticks between note change minus one [0, 31]
              ||└------> 0: auto repeat, 1: trigger with note
              |└-------> 1: skip 3rd note, 0: play 3rd note
              └--------> [reserved]

Old docs (these are only partially visible on the page because of markup errors, you need to open the raw view to read the complete content of the cells):

Next to the current playing note, play a second
and third note *[__X__]* for every *[__Y__]* ticks.

*[__X__]* includes 2 parameters: AAAABBBB, where
AAAA = base + amount to second note and
BBBB = second note + amount to third note.

*[__Y__]* includes 4 parameters: FEDttttt,
where F = reserved, E = toggle no third note,
D = toggle retrigger
ttttt = tick amount.

**_Note:_** Arpeggio is used for playing 3 notes out of a chord indivually

The offending song uses ATM_CMD_M_ARPEGGIO_ON(221, 97) which means 0xdd, 0x61, first byte is the notes offset in semitones so 2nd and 3rd note are 13 semitones up from the base note. Second byte says to play a new note every 2 ticks and don't play the 3rd note. Which means it will alternate between base_note and base_note+13 semitones. Second byte also sets bit 5. bit 5 is documented in the old docs as "toggle retrigger" (it doesn't say exactly what bit states 0 and 1 mean nor what exactly retrigger means).

The original code restarts the arpeggio on note-ON when bit 5 is set but lets the arpeggio continue until stopped explicitly independently of bit 5.

The new code plays a chord on note-ON when bit 5 is set (like the old code) but when bit 5 is reset it plays the chord once i.e. automatic chord re-trigger is disabled.

My reasoning was as follows: The docs were not clear in explaining how the effect should work so I looked at the code and saw that the chord was restarted when bit 5 was set but when bit 5 was reset it was ignored resulting in the behaviour outlined above. I figured that, as implemented, the bit wasn't useful because the original behaviour can be implemented without using a dedicated bit at all. So I concluded that there was probably a bug and the intention was to differentiate between chords that play once on every note-ON and chords that keep playing and that's what I have implemented in the current library.

We can fix this in at least two ways:

  1. Change the UI to state more clearly what the tick box means (e.g. "auto re-trigger" vs "note triggered") and set or reset bit 5 according to the new implementation.

  2. Change the library to swap the meaning of 0 and 1 for bit 5 and state more clearly what the tick box means (e.g. "auto re-trigger" vs "note triggered").

Comments?

jaygarcia commented 6 years ago

@dxxb thank you very much for this.

I'm OK w/ changing the UI, but that also means digging into ATMLib.js and I've got zero bandwidth at this time.

@slemmon please work to update the UI and I'll try to find time to update the JS Synth in 2018.

dxxb commented 6 years ago

For a quick fix change:

ATM_CMD_M_ARPEGGIO_ON(221, 97), \

to:

ATM_CMD_M_ARPEGGIO_ON(221, 65), \

it will change the arpeggio to be continuous in your song. Tell me if/how I can help you further.

jaygarcia commented 6 years ago

@slemmon can you get this to work properly in ShowCodeNew tomorrow?

I'm going to need to heavily rely on arpeggiator to reduce music size.

slemmon commented 6 years ago

PR: https://github.com/ModusCreateOrg/trackerEditor/pull/37

@jaygarcia I'm not 1000% sure I follow yet how this all will work but I think for the moment this change will allow the UI to keep working like it's been working and export the arpeggio command expected by lib2 for player playback.

If I've missed the mark I'll rework it tomorrow once you, me, and dxxb are online.

dxxb commented 6 years ago

I'm going to need to heavily rely on arpeggiator to reduce music size.

FYI, the arpeggiator also supports alternating between 1 note and silence (equally spaced), see notecut. Mentioning it just in case.

PR: #37

I see you changed the value assembled by the UI. That's one way to do it. I think changing the text associated with that FX option is essential to spare confusion on the user part. You could even get away with leaving the code as is with the right wording in the UI.

jaygarcia commented 6 years ago

@slemmon it doesn't work at all. Have you tested on device? Try the below atm file for something that is sonically more pleasing. arpeggiator2.atm.txt

dxxb commented 6 years ago

@jaygarcia, could it be a problem with the library instead? I don't have an arduboy with me right now so I cannot test it myself. Does changing manually:

ATM_CMD_M_ARPEGGIO_ON(221, 97), \

to:

ATM_CMD_M_ARPEGGIO_ON(221, 65), \

solve the issue?

jaygarcia commented 6 years ago

@dxxb i'm working on that now. I am not good w/ numbers to binary so I am working on it by hand.

ATM_CMD_M_ARPEGGIO_ON(0b00110100, 0b01000001),

jaygarcia commented 6 years ago

@dxxb yeah it's an issue with the library (i think).

ATM_CMD_M_ARPEGGIO_ON(221, 65), \ <-- Still fails

jaygarcia commented 6 years ago

@dxxb here is the header file arpeggiator.h.txt

dxxb commented 6 years ago

I just noticed this ATM_CMD_M_SET_TEMPO(5) tempo below 8 is not supported https://github.com/moduscreate/ATMlib/issues/15 :(

jaygarcia commented 6 years ago

Thanks @dxxb

I changed that block to the following:

My interpretation: (Arg 1)

(Arg 2) 0b00000001


/* pattern (channel) / bytes = 10*/
#define arpeggiator_pattern0_data { \
    ATM_CMD_M_SLIDE_VOL_ON(110), \
    ATM_CMD_M_ARPEGGIO_ON(0b00110100, 0b00000001), \
    ATM_CMD_M_SET_TEMPO(20), \
    ATM_CMD_M_CALL(4), \
    ATM_CMD_M_SET_LOOP_PATTERN(0), \
    ATM_CMD_I_STOP, \
}

Docs for reference :D

Arpeggio - Play a second and optionally third note after each played note

Macros         : ATM_CMD_M_ARPEGGIO_ON(p1, p2)
               : ATM_CMD_I_ARPEGGIO_OFF

Parameter count: 2

P1
    Size   : 1 byte
    Name   : Chord note shifts
    Format : bkkkknnnn
              ||||└└└└-> 3nd note shift: semitones to add to the 2nd [0,14]
              └└└└-----> 2nd note shift: semitones to add to the 1st [0,14]

P2
    Size   : 1 byte
    Name   : Effect configuration
    Format : b-edttttt
              |||└└└└└-> ticks between note change minus one [0, 31]
              ||└------> 0: auto repeat, 1: trigger with note
              |└-------> 1: skip 3rd note, 0: play 3rd note
              └--------> [reserved]
dxxb commented 6 years ago

I just noticed this ATM_CMD_M_SET_TEMPO(5) tempo below 8 is not supported moduscreate/ATMlib#15 :(

Even so, you should be able to hear something. The pattern should alternate between two notes (the 3rd note is disabled when bit 6 is 0). Notes should be 2 ticks apart so: note-on, delay 2 ticks, 2nd note, delay 2 tick, 1st note.

dxxb commented 6 years ago

ticks between note change minus one setting ticks to 1 means 2 ticks i.e. every other tick.

jaygarcia commented 6 years ago

Thanks. Got this working but am going to do it manually until we get the UI to do what it needs to do. =)