-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
audio: add a pitch-shifting feature #11214
Conversation
7e5e9e6
to
2b448fb
Compare
f0d6eea
to
74a2d95
Compare
Needs rebase and interface changes update. |
d52cd77
to
7c4b471
Compare
Download the artifacts for this pull request: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to have been forgotten, but works well and is potentially useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I forgot something as always. Could you put the old bindings for the curly braces in restore-old-bindings.conf
?
I would prefer to keep changing the speed with {} as it's way more useful than changing pitch. |
Yeah I think this is too niche to have a default keybind, users can configure it themselves if they want this feature |
9fdd0b4
to
64729be
Compare
keybinding changes have been taken out |
player/audio.c
Outdated
double drop = 1.0; | ||
|
||
if (!mpctx->opts->pitch_correction) { | ||
resample *= speed; | ||
speed = 1.0; | ||
} | ||
|
||
speed /= mpctx->opts->playback_pitch; | ||
|
||
if (mpctx->display_sync_active) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this expected this new feature will not work correctly with -adrop and -tempo sync modes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is to be expected.
-tempo prevents resampling and forces the pitch to be corrected.
-adrop prevents both resampling and the time-stretcher.
EDIT: actually, for -adrop, we could probably add something like this:
- drop *= speed * resample;
- resample = speed = 1.0;
+ drop *= speed * mpctx->speed_factor_a;
+ resample = mpctx->opts->playback_pitch;
+ speed = 1.0;
5e34c09
to
67a07dd
Compare
Just last thing is missing, note in |
player/audio.c
Outdated
break; | ||
default: goto normal; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this goto
here. How about
int video_sync = mpctx->display_sync_active ? mpctx->video_out->opts->video_sync : VS_NONE;
switch (video_sync) {
...
and put
resample *= pitch;
speed /= pitch;
into default case.
Uses resampling in tandem with a time-stretching audio filter to change the audio's pitch while leaving its tempo intact.
Uses resampling in tandem with any time-stretching audio filter (scaletempo, scaletempo2, rubberband) to change the audio's pitch while leaving its tempo intact.