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
561 stars 89 forks source link

pitch/frequency r2/r3 #67

Closed georgalis closed 2 years ago

georgalis commented 2 years ago

In prior versions, backward compatibility support was advertised... but I have held back to 1.9.0 because: 2.0.2 does not recognise flac, 1.9.0 works #59 and the new features did not justify rewriting the workflow wrapper.

Now in 3.0.0 it looks like the --help/--full-help has not been revised with regard to r3 pitch -p, --pitch Raise pitch by X semitones, or -f, --frequency Change frequency by multiple X

https://breakfastquay.com/rubberband/code-doc/classRubberBand_1_1RubberBandStretcher.html#af0afa0bd0e6752ac1ba40f2ad87ee559

"To put this in musical terms, a pitch scaling ratio corresponding to a shift of S equal-tempered semitones (where S is positive for an upwards shift and negative for downwards) is pow(2.0, S / 12.0)."

If I understand correctly, in r3 --pitch is now used to specify --frequency?

This may seem a trivial invocation time parameter, however given 1000s of tracks named according to invocation parameters used to render/reproduce them (with the old r2 engine), and the very confusing pitch now referencing frequency, and the inability to identify the version included in programs such as ffmpeg:

ffmpeg -h filter=rubberband

and the rather awkward situation of converting the old pitch parameter value to the new pitch parameter value without floating point (eg shell script), altogether supports my feature request of supporting the former pitch parameter in the r3 engine.

I very much would like to try r3 for higher quality transforms, but the consequence of removing/changing the pitch floating point calculation vs r2 creates a rather convoluted support requirement for tools using the api. Perhaps the former pitch/frequency specification could be restored with 3.0.1?

Given the challenges of getting transcoding software bootstrapped for non technical users, such as musicians, perhaps docker images should be used to insure musicians have compatible api installed when they try transcoding transforms? Is there a better method? Or, the pitch/frequency api from r2 could be ported to r3?

cannam commented 2 years ago

Hi George - I'm not totally clear on the question here but I think that it is probably a result of slightly unfortunate choice of names in the original program, and not a change or a compatibility issue.

It has always been the case that -p or --pitch specifies a pitch change in semitones (in the command-line tool) while -f or --frequency specifies a frequency multiplier. This continues to be the case with the 3.0 release regardless of which engine is in use.

Meanwhile, it has also always been the case that RubberBandStretcher::setPitchScale (in the library API) specifies a frequency multiplier and not a semitone value. That is, the API provides only the equivalent of --frequency but refers to it as pitch scale, and the command-line tool therefore performs a pitch-to-frequency conversion when you use the --pitch option.

Neither of these things has changed in 3.0 - both the command-line tool and the API should be totally backward compatible in this respect. The issue, I believe, is only that the naming is inconsistent between tool and API, but it always has been, and (for compatibility reasons!) that isn't going to change either.

If you find that the tool is in fact producing differently-pitched output than any previous version, please do report that as a bug (and if this is the case already, and I am misunderstanding this report, please do say so).

georgalis commented 2 years ago

Hi Chris, Thanks for clearing that up. I was seeing the following when I searched for the error, and found the linked r3 setPitchScale() doc (I did not see equivalent r1 or r2 doc?). Anyway with the following CLI error, it certainly looks like the API changed (my stderr is wrapped in ^^), suggestions?

^^ /Users/geo/k/5/b/3/9/8/0792cdf2ccc^/rubberband-1.9.0-gpl-executable-macos/rubberband -q --time 0.953743 --pitch -1 --crisp 5 -F ./@/tmp/_^3P3g8qxZwsA.opus-to191-ln.flac ./@/tmp/_^3P3g8qxZwsA.opus-to191-t0.953743-p-1-c5-F-ln.wav~ ^^
^^ Using time ratio 0.953743 and frequency ratio 0.943874 ^^

...success, but with r3 (3.0.0)

^^ /opt/homebrew/bin/rubberband -q --time 0.953743 --pitch -1 -finer -F ./@/tmp/_^3P3g8qxZwsA.opus-to191-ln.flac ./@/tmp/_^3P3g8qxZwsA.opus-to191-t0.953743-p-1-r3-F-ln.wav~ ^^
^^ Using time ratio 0.953743 and frequency ratio 0 ^^
^^ RubberBand: WARNING: Pitch scale must be greater than zero! Resetting it to default, no pitch shift will happen: 0 ^^

...obviously the r3 invocation, with --pitch -1 isn't going to have the desired effect?

georgalis commented 2 years ago

oh wait! it looks like I had a syntax error! s/-finer/--fine/ in the CLI fixed it!

^^ /opt/homebrew/bin/rubberband -q --time 0.953743 --pitch -1 --fine -F ./@/tmp/_^3P3g8qxZwsA.opus-to191-ln.flac ./@/tmp/_^3P3g8qxZwsA.opus-to191-t0.953743-p-1-r3-F-ln.wav~ ^^
^^ Using time ratio 0.953743 and frequency ratio 0.943874 ^^

...now I need to re-calibrate my levels because the true peaks are higher! That's a good thing (tm). šŸ‘šŸ¼