breakfastquay / rubberband

Official mirror of Rubber Band Library, an audio time-stretching and pitch-shifting library.
http://breakfastquay.com/rubberband/
GNU General Public License v2.0
580 stars 93 forks source link

Clicks while changing pitch shift factor #30

Closed danieleghisi closed 3 years ago

danieleghisi commented 4 years ago

Hello, and first of all thanks for this amazing library.

I am having troubles in changing the pitch shift factor dynamically, I am not sure I'm doing the right steps. I need to do it off-line, but if I understand correctly, dynamic change of shift is only possible in RealTime mode, hence I:

Is there something I should do to smooth things in this case? What is the correct way to implement a dynamic pitch shifting? (I have tried all the options I could find, without success.)

Thanks in advance. I'm putting a simplified version of my code below.

Daniele

`
RubberBandStretcher rb((long)sr, channelcount, options, 1., initial_pitch_scale); rb.setExpectedInputDuration(framecount);

// STUDYING....
    for (long i = 0; i < numblocks; i++) {
        new_pitch_scale = ....;             // <get the block pitch scale>
        rb.setPitchScale(new_pitch_scale);

        long count = ((i+1)*blocksize > framecount ? framecount - i*blocksize : blocksize);
        for (long c = 0; c < channelcount; c++) {
            for (long j = 0; j < count; j++)
                ibuf[c][j] = sample[(i*blocksize+j)*channelcount + c];
        }
        rb.study(ibuf, count, i==(numblocks-1));
    }

    // PROCESSING...
    long outframecount = 0;
    double old_pitch_scale = -1;
    long output_sample_wk_allocated_frames = framecount;
    float *output_sample_wk = (float *)bach_newptr(channelcount * output_sample_wk_allocated_frames * sizeof(float));
    for (long i = 0; i < numblocks; i++) {
        fill(ibuf, ...) // fill buffer from my samples

        new_pitch_scale = ....;             // <get the block pitch scale>
        rb.setPitchScale(new_pitch_scale);

        rb.process(ibuf, count, i==(numblocks-1));

        int avail = rb.available();
        if (avail > 0) { // we've got something as output
            float **obf = new float *[channelcount];
            long pivot = outframecount;
            for (long c = 0; c < channelcount; c++) {
                obf[c] = new float[avail];
            }
            rb.retrieve(obf, avail);
            outframecount += avail;

    /// then add the samples
            for (long c = 0; c < channelcount; c++) {
                for (int i = 0; i < avail; i++) {
                    output_sample_wk[(pivot + i)*channelcount + c] = obf[c][i];
                }
            }

            for (long c = 0; c < channelcount; c++)
                delete[] obf[c];
            delete[] obf;
        }
    }`
cannam commented 4 years ago

Hi Daniele - thanks for the note.

First, in real-time mode there is no need to call study - this function is there so that the offline mode can adjust timings so as to get the closest possible positioning for transients and for the end of the stream, but in real-time mode it has no opportunity to do that ahead of time anyway.

About the output quality, a useful thing may be if you are able to send to me, at cannam@all-day-breakfast.com (or attach here, but I imagine you may prefer to send offline) an example of the output you are getting. I should be able to use that to determine what kind of problem this is, and to follow up here accordingly.

Thanks!

cannam commented 4 years ago

Many thanks for sending the test examples. I think this is not a bug in your code, but rather a tricky problem in the shifting output itself - I have had a report of this before, have just set up a test rig for it, and am about to look into it in detail.

danieleghisi commented 4 years ago

Thank you so much, Chris!

Il giorno gio 10 set 2020 alle ore 18:25 Chris Cannam < notifications@github.com> ha scritto:

Many thanks for sending the test examples. I think this is not a bug in your code, but rather a tricky problem in the shifting output itself - I have had a report of this before, have just set up a test rig for it, and am about to look into it in detail.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/breakfastquay/rubberband/issues/30#issuecomment-690446277, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEO7O2CXA7GZMHTRZSTMX63SFD4WBANCNFSM4RCPIXQA .

cannam commented 4 years ago

I've just made a note of this in the new tracker at https://todo.sr.ht/~breakfastquay/rubberband/6 which I'm trying out - will add some further remarks there as I go along (and report back here after also).

danieleghisi commented 4 years ago

Thank you Chris, once again.

Il Mar 15 Set 2020, 15:37 Chris Cannam notifications@github.com ha scritto:

I've just made a note of this in the new tracker at https://todo.sr.ht/~breakfastquay/rubberband/6 which I'm trying out - will add some further remarks there as I go along (and report back here after also).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/breakfastquay/rubberband/issues/30#issuecomment-692719698, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEO7O2H2FDH45XOQUSU47SLSF5UYPANCNFSM4RCPIXQA .

cannam commented 4 years ago

Hi again - I believe this should be fixed now - please see details at https://todo.sr.ht/~breakfastquay/rubberband/6. The fixes are in the default branch; although the Mercurial repo is the authoritative one, the Github master branch also tracks it and contains the same fixes. I'm still doing some further testing, but would appreciate feedback (here or on the other tracker).

Note that these fixes apply only to builds using libsamplerate; there will still be problems with the other resampler libraries. I believe libsamplerate is the best choice anyway for almost all applications and would recommend using it.

danieleghisi commented 4 years ago

Hi Chris, thanks so much for the fix. I'll look into it as soon as possible and give you feedback.

Il giorno lun 21 set 2020 alle ore 18:08 Chris Cannam < notifications@github.com> ha scritto:

Hi again - I believe this should be fixed now - please see details at https://todo.sr.ht/~breakfastquay/rubberband/6. The fixes are in the default branch; although the Mercurial repo is the authoritative one, the Github master branch also tracks it and contains the same fixes. I'm still doing some further testing, but would appreciate feedback (here or on the other tracker).

Note that these fixes apply only to builds using libsamplerate; there will still be problems with the other resampler libraries. I believe libsamplerate is the best choice anyway for almost all applications and would recommend using it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/breakfastquay/rubberband/issues/30#issuecomment-696215455, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEO7O2GLEOJT2XZWUPNBEX3SG53BJANCNFSM4RCPIXQA .

danieleghisi commented 4 years ago

Hi Chris,

fantastic. I've looked into it and recompiled. I can confirm that with libsamplerate the clicks are gone, thanks so much.

There is a little amplitude modulation going on, not huge but hearable. Let me know if you want me to send you audio examples, I can do that privately via email. I suspect that it has to do with windowing because if I set the window size to "long", the modulation slows down significantly. To be honest, I'm not 100% sure that wasn't already the case before – I was wondering whether that was the crossfading, but now that I think about it, it must have been already there, as there is also some modulation when I use speex as resampler (unfortunately I'm really not a DSP expert so I cannot be of much help here...)

Thanks again for all your help and your work – and thanks for having made RubberBand available to everybody.

Daniele

Il giorno lun 21 set 2020 alle ore 22:40 Daniele Ghisi < danieleghisi@gmail.com> ha scritto:

Hi Chris, thanks so much for the fix. I'll look into it as soon as possible and give you feedback.

Il giorno lun 21 set 2020 alle ore 18:08 Chris Cannam < notifications@github.com> ha scritto:

Hi again - I believe this should be fixed now - please see details at https://todo.sr.ht/~breakfastquay/rubberband/6. The fixes are in the default branch; although the Mercurial repo is the authoritative one, the Github master branch also tracks it and contains the same fixes. I'm still doing some further testing, but would appreciate feedback (here or on the other tracker).

Note that these fixes apply only to builds using libsamplerate; there will still be problems with the other resampler libraries. I believe libsamplerate is the best choice anyway for almost all applications and would recommend using it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/breakfastquay/rubberband/issues/30#issuecomment-696215455, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEO7O2GLEOJT2XZWUPNBEX3SG53BJANCNFSM4RCPIXQA .

daschuer commented 3 years ago

We use librubberband with Mixxx and libsamplerate.

We are not suffering the original bug regarding speex, but I can confirm the audio glitches in current master and librubberband 1.8.2. There is no notable difference for me.

Here a 440 Hz track recorded with Mixxx at 50 % pitch:

grafik

Here another example moving the pitch slider from -50 % to + 50 %

grafik

This can be heard well. It does also not happen at unity gain.

Fortunately I cannot really hear the issue with real music.

daschuer commented 3 years ago

The Mixxx bug is here: https://bugs.launchpad.net/mixxx/+bug/1909938

cannam commented 3 years ago

@daschuer I can't tell from your images what effect you are getting - it looks like either gaps (which would be surprising) or amplitude modulation as Daniele describes (which may be expected, depending on the circumstances).

I've filed a tracker here https://todo.sr.ht/~breakfastquay/rubberband/11 to monitor this, as I'll be looking at the general subject of correctness during live pitch shifting again post-1.9.1 (when I've got the build system out of the way).

I'm going to close this issue because it's confusing to have it change topic mid-stream, but if you have any specific test cases or want to comment on the upstream issue mentioned above, I'd be happy to hear it!