Provide custom loop ranges in SampleEvent

igorski commented 5 years ago

@robtize Can you try this feature branch for size ? One hand, I also appreciate if you can verify whether existing functionality still behaves the same (luckily this is easy as it will become apparent when your songs start sounding strange). The reason I'm asking is that I have moved some BaseAudioEvent functionality into SampleEvent (because quite frankly, it was unique to the subclass).

You should now be able to specify a loop start offset using SampleEvent.setLoopStartOffset() (which will automatically enable looping). When you are playing a SampleEvent, as soon as it reaches its end, it will start looping from designated loop start offset. Though we talked about a loop end offset, I am not entirely sure if it is needed at this point (as no release is specified). Or do you have an example use case for it ?

robertcapsure commented 5 years ago

I'm having trouble playing loop able events that are added to the sequencer

igorski commented 5 years ago

What is the exact problem with the loopable events ? There was a known issue in this pull request where loopeable events that where sequenced would play where the loop left off instead of at the sample start (when the sequence in which the events were positioned started its loop). This has been addressed in commit 775253c (added at a later stage in this PR, maybe your branch is behind?).

robtize commented 5 years ago

Yes I’m on the latest commit (775253c https://github.com/igorski/MWEngine/commit/775253c7c8a99dbfcf5df9cc94cee8b0157fc99d) but still seem to have that exact problem still. Maybe I’m Implementing it wrong?

On Fri, Nov 16, 2018 at 7:52 AM Igor Zinken notifications@github.com wrote:

What is the exact problem with the loopable events ? There was a known issue in this pull request where loopeable events that were sequenced would play where the loop left off instead of at the sample start (when the sequence in which the events were positioned started its loop). This has been addressed in commit 775253c https://github.com/igorski/MWEngine/commit/775253c7c8a99dbfcf5df9cc94cee8b0157fc99d (added at a later stage in this PR, maybe your branch is behind?).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/igorski/MWEngine/pull/88#issuecomment-439436577, or mute the thread https://github.com/notifications/unsubscribe-auth/Ahi8FrRWB3RG_f3j1MVH3EmtBJc3OapZks5uvt8mgaJpZM4YhOON .

igorski commented 5 years ago

@robtize do you still experience problems with this branch or can it be merged ? If not, what is the exact issue so I can have a peek at what might be amiss?

robtize commented 5 years ago

Hey Igor!

Yes I’m still experiencing this problem where loopable events work when I play them live but they don’t work when they’re added to the sequencer. Also the mono loopable events are only playing from one channel.

On Thu, Nov 29, 2018 at 2:12 AM Igor Zinken notifications@github.com wrote:

@robtize https://github.com/robtize do you still experience problems with this branch or can it be merged ? If not, what is the exact issue so I can have a peek at what might be amiss?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/igorski/MWEngine/pull/88#issuecomment-442778918, or mute the thread https://github.com/notifications/unsubscribe-auth/Ahi8Fq-aIAgcGL0ekRvJho33MG27POgbks5uz7MQgaJpZM4YhOON .

igorski commented 5 years ago

OK, that's funny: I had to write a custom summing routine for the SampleEvents using a custom playback rate (for pitching samples to the appropriate sample rate). In this routine I made the same error as before in the base routine (where mono events would be written to only a single channel of the output). Events with mono buffers and the before mentioned properties should now be spread across the stereo field.

The issue with sequenced event looping should be resolved, could you pull the branch and have another check ? :)

robtize commented 5 years ago

Hey Igor!

The mono to stereo works now however I’m still getting the same problem when adding the events to a sequencer :(

On Sat, Dec 1, 2018 at 1:15 PM Igor Zinken notifications@github.com wrote:

@igorski commented on this pull request.

In src/main/cpp/events/sampleevent.cpp https://github.com/igorski/MWEngine/pull/88#discussion_r238076803:

@@ -462,62 +458,39 @@ void SampleEvent::mixBuffer( AudioBuffer outputBuffer, int bufferPosition, tgtBuffer[ i ] += (( s1 + ( s2 - s1 ) frac ) * _volume ); } }

else if ( loopStarted && fi >= lo )

{

Single conditional on fBufferPointer assignment can remove this entire branch (which is mostly duplicate code anyways).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/igorski/MWEngine/pull/88#pullrequestreview-180538555, or mute the thread https://github.com/notifications/unsubscribe-auth/Ahi8FtfXr8ElVtnpstwA6dbi2d3Ublpyks5u0vGGgaJpZM4YhOON .

igorski commented 5 years ago

Doh!

Can you send an example event that isn't working correctly?

I'm thinking sequencer tempo, amount of measures and the SampleEvents source buffer length (and event playback rate or source sample rate and engine sample rate), even start, end and length values as well as the loop range values. If you can paste snippets of how you assign these values via the setters that's be great as well :)

robtize commented 5 years ago

Hey Igor!

Sorry for the late response here's the data that was requested:

Tempo = 120 Measures = 32 Source Buffer Length = 31748 Event start = 0 Event end = 31748 loopstart = 515 Engine samplerate = 44100 Sample's samplerate = 44100

This is just for the events added to sequencer

igorski commented 5 years ago

Is the loop start value correct ? A value of 515 at a 44.1 kHz sample rate would equal a duration of 0.011 seconds, making it hard to hear whether the sample is playing from the beginning or the set loop start offset as these two points are so short apart. Can you describe the exact problem that you're experiencing in case I'm thinking of the wrong thing?

robertcapsure commented 5 years ago

Ok bad example lol let’s try this one With the same results:

Sample length - 115872 Loop start - 760621

On Tue, Dec 4, 2018 at 9:39 AM Igor Zinken notifications@github.com wrote:

Is the loop start value correct ? A value of 515 at a 44.1 kHz sample rate would equal a duration of 0.011 seconds, making it hard to hear whether the sample is playing from the beginning or the set loop start offset. Can you describe the exact problem that you're experiencing in case I'm thinking of the wrong thing?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/igorski/MWEngine/pull/88#issuecomment-444189264, or mute the thread https://github.com/notifications/unsubscribe-auth/Aelp2SI0o7mm6PsMOtsnS4Fc48rwLEvzks5u1rNHgaJpZM4YhOON .

robertcapsure commented 5 years ago

The exact problem is sounds work fine when playing them live but when I add them to the sequencer I hear maybe the start of one sample then nothing for the duration of the sequence.

On Tue, Dec 4, 2018 at 9:44 AM Robert Avellar robert@capsureit.com wrote:

Ok bad example lol let’s try this one With the same results:

Sample length - 115872 Loop start - 760621

On Tue, Dec 4, 2018 at 9:39 AM Igor Zinken notifications@github.com wrote:

Is the loop start value correct ? A value of 515 at a 44.1 kHz sample rate would equal a duration of 0.011 seconds, making it hard to hear whether the sample is playing from the beginning or the set loop start offset. Can you describe the exact problem that you're experiencing in case I'm thinking of the wrong thing?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/igorski/MWEngine/pull/88#issuecomment-444189264, or mute the thread https://github.com/notifications/unsubscribe-auth/Aelp2SI0o7mm6PsMOtsnS4Fc48rwLEvzks5u1rNHgaJpZM4YhOON .

igorski commented 5 years ago

Found the root cause! The floating point units used to track progress of pitched samples would lead to an inaccurate tracking of when the sample is "retriggered". Please pull again, I have also updated the example Activity (temporarily) to provide an example of how to test this:

All synth events have been removed, you will only hear a sequenced sample that counts from 1 to 8, every half second. This sample is configured to loop its contents from the halfway point (thus after counting to 8, the loop will start from the 5 count, e.g. "1, 2, 3, 4, 5, 6, 7, 8, 5 < LOOP START, 6, 7..." The live event button triggers a live event playing back the same sample with the same loop range. The sequenced sample is triggered on the 0, the sequencer will loop a single measure, so you will hear it count from 1 to 4 (with a bit of 5 as I didn't time my counts super accurately :)). You can use the "drum pitch" slider to shift the pitch of both the sequenced as the live sample up or down (to test at different playback rates).

With the above in mind you can observe the following behaviour:

When the sequencer loops, the sample should start again from the beginning (at the "1" count), we call this the retrigger.
Even when shifting the pitch, you should always hear the sample playing from the beginning when the sequencer starts it loop.
When pitching up, you will hear more of the sample as it plays back faster, when pitching down, you will hear less as it plays slower, thus the sample will always play all the content it can within the sequencers loop range, just scaled to ratio.
When lowering the sequencer speed (while having the sample pitched all the way up), you will hear the sample loop from the "5 count" after it has played back in full, it will keep repeating from the "5 count" until the sequencer finally gets to loop again.

(BTW: in your second example you provided a loop start that exceeds the range of the sample, I'm pretty sure that was a typo, but just thought I'd throw it out there :))

robertcapsure commented 5 years ago

Doh!

hahahha i meant loop start 76021 not 760621. damn it lol. but thanks for this!! going to try it as soon as I get home!

robtize commented 5 years ago

so i'm actually still having the same issue :( i narrowed down the bug a little more and this is exactly what's happening.

if a sample event has setLoopeable = true and added to sequencer

the sample event will only play the full duration you set of the event and not loop if the start position is 0 if the sample event is any position > 0 it will not play at all

this is all true even if these properties are not set: SampleUtility.pitchShift setLoopStartOffset setEventLength

igorski commented 5 years ago

That was a very helpful pointer as I could not have found the issue (I was naively testing with events starting at the 0 offset, while I actually refactored the base event class assuming I left it unbroken!). Updated the branch again (also extended the example activity to be multi measure in length and have events start halfway through the first measure).

While creating my tests, I also confused myself a few times so it might be helpful to illustrate my steps as there can be confusion between a SampleEvent and the AudioBuffer (the sample) it represents.

Let's assume that the events sample has a total (unlooped) length of 5000 samples, when assigning a sample to a sample event, it's event length will by default equal the length of its sample buffer
When setting the start position, the end position is calculated to be equal to: start position + total event length (5000 samples)
When setting loopeable (either with or without custom loop start offset), the event length remains unchanged. As such you will not actually hear a loop occur just by toggling this property!
You must explicitly set the new length of the event to specify for how long it should play (as basically the event describes when and for how long a sound should play, regardless of whether it is synthesized, has a sample, or even a loops the sample).

For instance setting the events length to 7500 samples will automatically adjust the event end and lead to the following behaviour:

Event starts playing when event start is reached by Sequencer
Event plays for 5000 samples after which it reaches the end of its sample
Loop of the events sample now starts, event starts playing its sample again from the specified loop offset (let's say this was 1000)
Event continues playing for another 2500 samples (to reach the new event length of 7500 samples).

This implies the event has played the following ranges of its sample:

0 to 5000 | 1000 to 3500

robertcapsure commented 5 years ago

hey igor! thanks for this! it seems like we're almoosssttt there!

i'm able to play loopable events at any position in the sequencer and they're looping. however doesn't seem like the loops are starting back at the loopoffsets. once again this is only for events added to the sequencer. events playing live work completley fine!

hope fully this code helps a little on how we're setting things up

    public static TZSampleEvent getEvent(SampledInstrument instrument, String soundName, int note) {
        TZSample sample = noteToSampleMap.get(soundName + note);
        if (sample == null) {
            return null;
        }
        loadToBuffer(soundName, note);

        final SampleEvent sampleEvent = new SampleEvent(instrument);
        String sampleName = soundName + " " + sample.rootKey;
        sampleEvent.setSample(SampleManager.getSample(sampleName), SampleManager.getSampleRateForSample(sampleName));
        float pitchShiftAmount = note - sample.rootKey;
        if (sample.tune != 0) {
            pitchShiftAmount += sample.tune / 100f;
        }
        if (sample.loopStart > 0) {
            sampleEvent.setLoopeable(true);
            sampleEvent.setLoopStartOffset(sample.loopStart);
            sampleEvent.setSampleLength(sample.loopEnd);
        }
        SampleUtility.pitchShift(sampleEvent, pitchShiftAmount);
        return new TZSampleEvent(sample, sampleEvent);
    }

    private void addEventToSequencer(int instrumentIndex, String soundfont,
                                     int note, float position, float duration, boolean isMute) {
        SampledInstrument instrument = instruments.get(instrumentIndex);
        TZSampleEvent tzSampleEvent = TZSampleManager.getEvent(instrument, soundfont, note);
        if (tzSampleEvent == null) {
            return;
        }
        final SampleEvent sampleEvent = tzSampleEvent.getSampleEvent();

        String key = instrumentIndex + " " + note + " " + position;
        sampleEvent.setEventStart(BufferUtility.secondsToBuffer(position, SAMPLE_RATE));
        float totalDuration = (float) (tzSampleEvent.getSample().release * .1f) + duration;
        sampleEvent.setEventLength(BufferUtility.secondsToBuffer(totalDuration, SAMPLE_RATE));
        sampleEvent.addToSequencer(); // samples have to be explicitly added for playback
        sampleEvent.setEnabled(!isMute);

        sequencedEvents.put(key, tzSampleEvent.getSampleEvent());
    }

igorski commented 5 years ago

That was once more helpful, could you have another go with the latest version ? :)

robtize commented 5 years ago

THIS PR IS APPPRROOVVEEDDDD!!!! omg thank you sooo much! this may be the last requested until our release thanks sooo much! you have no idea what this does for us lol

robtize commented 5 years ago

I do have one more question. I know we talked about the clipping at the end or beginning of the loop was because of no crossfading. do you have any suggestions on how i can work around this? seems like almost every sound i have that loops either has some clipping or some artifact when it loops.

igorski commented 5 years ago

Great news! Will add some unit tests and revert the example Activity. As for the clipping: ideally you find a loop point where the first sample at the loop offset has an amplitude more or less equal to the last sample at the end of the loop, though this implies that you cannot have fadeouts in long samples (as there will be a large volume jump when the sample continues playing from an offset which has a high amplitude). Ideally you specify your loop ranges at the zero crossing point (where the sample's amplitude is 0). It is debatable whether the engine should provide an automated cross fading solution here as this would not always be desired (DAWs follow the same principle).

We could consider adding a utility that finds the nearest zero crossing for your specified loop ranges and adjusts the provided values accordingly, but there are probably external utilities for that purpose you can use to pre process your samples and the calculated loop ranges. I would suggest that further discussion on said topic would be in a separate issue though as this PR is quite big at this moment in time :)

igorski / MWEngine

Provide custom loop ranges in SampleEvent #88

@igorski commented on this pull request.