Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crossfade when crossing pitch 1.0 at non 1.0 speed #4030

Draft
wants to merge 1 commit into
base: 2.3
Choose a base branch
from

Conversation

daschuer
Copy link
Member

@daschuer daschuer commented Jun 28, 2021

This removes a crackling sound with a 440 Hz sine tone.
Fixes https://bugs.launchpad.net/mixxx/+bug/1933756

This removes a crackling sound with a 440 Hz sine tone.
Fixes lp1933756
@git-developer
Copy link
Contributor

git-developer commented Jun 28, 2021

Thanks for your quick reaction, I really appreciate that!

I downloaded the PR artifacts (mixxx-2.3-beta-4293-g2945f43418) for Ubuntu 20.04 and Windows, and made some tests using the sample attached to the bug report. Result:

  • Rubberband @ 11ms or 23 ms: No clicks (yay!). But unfortunately, signal is not stable, the demo sample (sinus wave) sounds as if tiny dropouts occur. Also high CPU load (well known Rubberband issue).
  • Soundtouch @ 11 ms: clicks, no difference to 2.3-beta
  • Soundtouch @ 23 ms: clicks, but seems less than 11ms
  • Soundtouch @ 46 ms: the same as 23 ms

I'm preferring Soundtouch to save resources for other tasks (controllers, vinyl control, fx, ...).

@daschuer
Copy link
Member Author

daschuer commented Jun 28, 2021

The solution I have implemented is crossfading from one setting to the other. This doubles the CPU load for one buffer, which may cause a buffer underrun which clicks as well.
The cross fading itself may produce a kind of flanger effect, that causes the loudness dip. This is specific to the sine wave frequency and shout not be an issue with real music.

Normally we play real Musik, so the question is if this improved the situation for that?
Or is the dropout risk too high?

@git-developer
Copy link
Contributor

With Soundtouch, CPU load is very low (deep green in LateNight skin, didn't look at top), and no buffer underruns have been reported in the Mixxx settings window. That makes me think a buffer underrun is unlikely as cause for the clicks. I will make a test with real music and report back.

@uklotzde uklotzde added this to the 2.3.1 milestone Jun 28, 2021
@git-developer
Copy link
Contributor

git-developer commented Jun 29, 2021

I made another test in a less synthetic scenario: real music, Soundtouch scaler, vinyl control.

First of all, I focused on cpu usage to rule out buffer underruns. The system uses a real-time kernel and is able to handle 5.8ms under normal conditions. For the test, latency has been set to 23ms. During the test, monitoring with top shows ~40% cpu and a load of 0.6. The colored cpu bar in mixxx is deep green.

The test:

  1. play a track,
  2. adjust the pitch via vinyl control to 2%
  3. reset the key

Resetting the key causes 2 changes: the usual Soundtouch artifacts (e.g. tiny parts of audio are doubled / omitted) and heavy clicks. Only the clicks are subject of this PR. They stay as long as nothing else is changed. When pitch is changed via vinyl control, the clicks go away and only the Soundtouch artifacts stay.

@git-developer
Copy link
Contributor

git-developer commented Jul 9, 2021

Let me add a few words about my real world scenario and why this bug is problematic for it. I'm using vinyl control without keylock normally which sounds OK most of the time. Sometimes the key of two tracks does not match, and a mix would sound really inharmonic. In this case, I adjust the key of the second track via a MIDI controller for 1 or 2 semi-tones for the mix. When the first track has ended and the second track has a break or a part with few or no melody, I reset the key via MIDI so that time-stretching is disabled again. I tried reset_key and pitch_set_zero for the key reset, but both suffer from the click problem.

There's one thing I noticed recently: I'm not sure if reset_key or pitch_set_zero are the right command for my scenario. What I really like to do is disable time-stretching engine. Is there a better command for this?

@git-developer
Copy link
Contributor

Is there a better command for this?

The correct command should be pitch_adjust_set_default.

  • reset_key applies the key that is stored with the track (at zero pitch), ignoring the actual pitch and tempo.
  • pitch_set_zero directly sets the total pitch of the track, overriding the rate slider and pitch_adjust.
  • pitch_adjust adjusts the pitch in addition to the rate slider. This should work in combination with vinyl control where the vinyl record controls the rate slider.

@uklotzde
Copy link
Contributor

uklotzde commented Sep 7, 2021

Rescheduling to 2.3.2.

@uklotzde uklotzde modified the milestones: 2.3.1, 2.3.2 Sep 7, 2021
@github-actions
Copy link

github-actions bot commented Dec 7, 2021

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Dec 7, 2021
@git-developer
Copy link
Contributor

The issue persists.

@daschuer
Copy link
Member Author

daschuer commented Dec 7, 2021

Yes, I am sorry. This PR is IMHO not in a merge-able state, because it doubles the CPU for the zero crossing cycle and does according your tests not solve the issue completely.
I keep ti open because it improves the situation bit and we can use it as a reference for future attempts.

I do not expect a solution anytime soon. The root issue is that we our use case seems to be not considered during the development of the sound stretch libraries.

We need to dive into the library code itself and work around the revitalization when crossing the zero point.
I think a good way forward would be to discuss it upstream.
https://sourceforge.net/p/soundtouch/discussion/
https://todo.sr.ht/~breakfastquay/rubberband
Maybe there is a workaround or we are doing something wrong in Mixxx.

I would be happy if you could make a start and post the links at Launchpad
https://bugs.launchpad.net/mixxx/+bug/1933756

@git-developer
Copy link
Contributor

No problem, my comment was just a reaction on the stale label. I'm currently not affected by the issue because I switched to pitch_adjust. Nevertheless the bug exists, so I added the links as requested.

@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Dec 8, 2021
@daschuer daschuer removed this from the 2.3.2 milestone Jan 15, 2022
@daschuer daschuer marked this pull request as draft January 15, 2022 22:33
@github-actions
Copy link

This PR is marked as stale because it has been open 90 days with no activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issues that haven't been updated for a long time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants