microsoft / pxt-arcade

Arcade game editor based on Microsoft MakeCode
https://arcade.makecode.com
MIT License
473 stars 200 forks source link

Browser: music.playMelody doesn't stay on the beat #1846

Closed kristianpedersen closed 4 years ago

kristianpedersen commented 4 years ago

I wanted to create a simple hihat pattern, but Chrome doesn't keep time very well.

If I change the BPM to 480, it's even more apparent.

const adsr = "@10,10,0,10 "
const wave = "~5 "
const notes = "c "
const output = adsr + wave + notes

forever(function () {
    music.playMelody(output, 120)
})

The screenshot below has a 120 BPM grid. Red: Output from MakeCode is clearly off beat Green: This is what I would have expected to get

image

I want to make my own chiptunes in MakeCode Arcade. It would be so much fun!

kristianpedersen commented 4 years ago

After trying some more, I came up with this solution.

It's not perfect, but it handles rapid notes much better. Do you have any suggestions for improvements?

let previousTime = 0
let offset = 0
let time = 0

const adsr = "@10,10,0,10 "
const wave = "~5 "
const notes = "c "
const output = adsr + wave + notes

forever(function updateClock() {
    if (game.runtime() - time >= 125 - offset) {
        time = game.runtime()
        offset = time % 125
    }
})

forever(function makeSound() {
    if (time != previousTime) {
        music.playSound(output)
        previousTime = time
    }
})
jwunderl commented 4 years ago

forever won't be able to keep it truly consistent, as it has an implicit pause at the end of each iteration to yield to other things (here). Does this work better for you: https://makecode.com/_dmhE7ACb3Chd ?

let adsr = "@10,10,0,10 "
let wave = "~5 "
let notes = "c "
let output = "" + adsr + wave + notes
const myMelody = new music.Melody(output);
myMelody.loop();

that should keep it more consistent?

kristianpedersen commented 4 years ago

Timing-wise, it works really well. This is how it compares to the grid:

image

Still, I can only play one melody at a time. The timing is good enough for me, but they're not playing simultaneously, and adding forever loops around the for loops causes timing issues.

// Hihat
const h = {
    adsr: "@10,10,0,10",
    wave: "~5 ",
    notes: "c-480",
}
const hihat = new music.Melody(h.adsr + h.wave + h.notes);

// Bass
const b = {
    adsr: "@10,100,0,10",
    wave: "~15 ",
    notes: "c2-240 c3-240",
}
const bass = new music.Melody(b.adsr + b.wave + b.notes);

for (let i = 0; i < 16; i++) {
    hihat.playUntilDone()
}

for (let i = 0; i < 4; i++) {
    bass.playUntilDone();
}
jwunderl commented 4 years ago

Hm, well that one won't work because the play until done is blocking, so it will play 16 'hihat's and then 4 'bass'es; the reason to use loop is because it will run continuously in the underlying music player, so it doesn't have the same issues with running on it's own. is https://makecode.com/_12LARa9x6ijH more what you want? You should be able to run 3 melodies in a loop at the same time (and if I remember right that's just an arbitrary limit we set that could potentially be increased, as long as things work properly on hardware too)

music is definitely one of the harder categories to interact with right now, and we're hoping to flesh it out a bit. I'll tag @riknoll for someone who knows a thing or two about music

kristianpedersen commented 4 years ago

I've made further improvements. It's still kind of inaccurate, but I guess it works:

https://makecode.com/_K8FiaFikRcPP

enum Waveform {
    triangle = 1,
    sawtooth = 2,
    sine = 3,
    noise = 5,
    square10 = 11,
    square20 = 12,
    square30 = 13,
    square40 = 14,
    square50 = 15,
}

const hihat = createMelody("c-480 c c c c c c c", [10, 10, 0, 10], Waveform.noise)

const bass = createMelody("c1-480 c0 c2 c0 c1 c0 c2", [10, 100, 0, 10], Waveform.square50)
const bass2 = createMelody("a1-480 a0 a2 a0 a1 a0 a2", [10, 100, 0, 10], Waveform.square50)
const bass4 = createMelody("g1-480 g0 g2 g0 g1 g0 g2", [10, 100, 0, 10], Waveform.square50)
const bass3 = createMelody("f1-480 f0 f2 f0 f1 f0 f2", [10, 100, 0, 10], Waveform.square50)

const arp = createMelody("c5-480 c0 b4 c0 a4 c0 g4 c0", [10, 50, 0, 20], Waveform.sawtooth)
const arp2 = createMelody("d5-480 c0 c4 c0 b4 c0 g4 c0", [10, 50, 0, 20], Waveform.sawtooth)

for (let repetition = 0; repetition < 2; repetition++) {
    for (let i = 0; i < 2; i++) {
        hihat.play()
        bass.play()
        arp.play()
        pause(1000)
    }

    for (let i = 0; i < 2; i++) {
        hihat.play()
        bass2.play()
        arp2.play()
        pause(1000)
    }

    for (let i = 0; i < 2; i++) {
        hihat.play()
        bass3.play()
        arp.play()
        pause(1000)
    }

    for (let i = 0; i < 2; i++) {
        hihat.play()
        bass4.play()
        arp2.play()
        pause(1000)
    }
}

function createMelody(notes: string, adsr: number[], waveform: Waveform) {
    const env = "@" + adsr.join(",")
    const wave = "~" + convertToText(waveform) + " "
    return new music.Melody(env + wave + notes)
}
riknoll commented 4 years ago

I'm hoping to find some time to improve this soon, but right now the only way to get accurate timing is to not use the melody blocks and instead use the underlying APIs directly. If you want more info, you can check out the "sound instructions" section of this page.

Here's some sample code that sorta shows the usage:

https://makecode.com/_4j4hkF95cMjd

That creates a sound with two envelopes, one for pitch and the other for the amp. The actual playing happens in the button handler in the bottom. The second argument for queueplayinstructions is a time delta that you can use to schedule things.

kristianpedersen commented 4 years ago

If you want more info, you can check out the "sound instructions" section of this page.

Thanks! Buffers are completely new to me, so I'll need to take some time to understand the examples in your code.

Below is my first buffer attempt, but music.playInstructions() is not found - is that why you've create a custom function under the music namespace in your example?

What else am I missing? I just chose the same buffer size you did, although the usage is probably different.

What would I do to create one note and repeat it evenly n times?

const test = control.createBuffer(96)
let bufferPosition = 0

test.setNumber(NumberFormat.UInt8LE, bufferPosition, 2)         // waveform
test.setNumber(NumberFormat.UInt8LE, bufferPosition + 1, 0)     // unused
test.setNumber(NumberFormat.UInt16LE, bufferPosition + 2, 1000) // frequency
test.setNumber(NumberFormat.UInt16LE, bufferPosition + 4, 1000) // duration (ms)
test.setNumber(NumberFormat.UInt16LE, bufferPosition + 6, 100)  // start volume
test.setNumber(NumberFormat.UInt16LE, bufferPosition + 8, 0)    // end volume
test.setNumber(NumberFormat.UInt16LE, bufferPosition + 10, 0);  // end frequency
// bufferPosition += 96?

music.playInstructions(test) // Property 'playInstructions' does not exist on type 'typeof music'. 

Also, just for fun, here's a graphical solution. The timing issues are the same, but it was fun!

https://makecode.com/_KzWX4h6saXiD

riknoll commented 4 years ago

ah yeah, sorry you need to have that declaration! If you copy and paste it from my code it should work fine. It's just a type definition for a C++ function.

kristianpedersen commented 4 years ago

The documentation calls it playInstructions(), but I only got it working using queuePlayInstructions().

I got it working though! This is fun! The timing is acceptable, and I can live without ADSR.

I don't like how I'm creating separate variables for each note. They all share the same characteristics otherwise. Maybe one buffer array for each sound, containing all possible note values?

https://makecode.com/_hLg9f8AK4WUY

enum Waveform {
    triangle = 1,
    sawtooth = 2,
    sine = 3,
    noise = 5,
    square10 = 11,
    square20 = 12,
    square30 = 13,
    square40 = 14,
    square50 = 15,
}

namespace music {
    //% shim=music::queuePlayInstructions
    export function queuePlayInstructions(timeDelta: number, buf: Buffer) { }
}

const bass = createSound(Waveform.square50, Note.A * 0.25, 100)
const e = createSound(Waveform.sawtooth, Note.E4, 100)
const g = createSound(Waveform.sawtooth, Note.G4, 100)
const b = createSound(Waveform.sawtooth, Note.B4, 100)
const d = createSound(Waveform.sawtooth, Note.D5, 100)
const hihat = createSound(Waveform.noise, 1000, 10)

let tick = 0
for (let i = 0; i < 4; i++) {
    for (let ms = 125 * 16 * i; ms < 125 * 16  + 125 * 16 * i; ms += 125) {
        if ([0, 1, 6, 7].some(n => n == tick)) {
            music.queuePlayInstructions(ms, bass)
            music.queuePlayInstructions(ms, e)
            music.queuePlayInstructions(ms, g)
            music.queuePlayInstructions(ms, b)
            music.queuePlayInstructions(ms, d)
        }

        if (tick == 10 || tick == 14) {
            music.queuePlayInstructions(ms, bass)
        }

        music.queuePlayInstructions(ms, hihat)
        tick++
        tick %= 16
    }
}

function createSound(wave: Waveform, frequency: number, duration: number, endFrequency?: number, startVolume?: number, endVolume = 0) {
    const b = control.createBuffer(96)
    b.setNumber(NumberFormat.UInt8LE, 0, wave)  // waveform
    b.setNumber(NumberFormat.UInt8LE, 1, 0)     // unused
    b.setNumber(NumberFormat.UInt16LE, 2, frequency)  // frequency
    b.setNumber(NumberFormat.UInt16LE, 4, duration)  // duration (ms)
    b.setNumber(NumberFormat.UInt16LE, 6, 100)  // start volume
    b.setNumber(NumberFormat.UInt16LE, 8, endVolume)    // end volume
    if (!endFrequency) {
        b.setNumber(NumberFormat.UInt16LE, 10, frequency) // end frequency
    } else {
        b.setNumber(NumberFormat.UInt16LE, 10, endFrequency) // end frequency
    }

    return b
}
riknoll commented 4 years ago

yeah, you'll need to make one for each note. I'm planning on improving this! I am currently working on a sound designer and sequencer but no ETA right now.

kristianpedersen commented 4 years ago

Really looking forward to trying it. Thanks for your help!