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
566 stars 91 forks source link

Remove real-time mode for the CLI #33

Closed Vulcanostrol closed 3 years ago

Vulcanostrol commented 3 years ago

Hi, I am quite new to this project, so I decided to make an issue first before immediately doing a pull request. I noticed the CLI provides a -R option to enter real-time mode. However, I believe streaming input and output is not supported with the CLI?

I do know that when using the C++ library, the real-time mode can be used, so I would suggest only removing the -R option to enter the mode for the CLI. The reason being that it can confuse CLI users (like myself).

Hopefully, I am not missing anything. For example, the CLI somehow does support streaming or that this is a planned feature.

cannam commented 3 years ago

Hi there - thanks for the query! You're correct that the CLI doesn't support any sort of streaming, and this is not a planned feature either.

The reason it has the -R option anyway, is that the library behaves differently in real-time mode even if you are not actually using it in real time. In real-time mode you get a single pass with causal transient detection, while the non-real-time mode is two-pass. The fundamental method is the same, but the output may still differ between the two.

So I think the option is worth having, especially since much of the command line interface is made up of fairly low-level options already. And this is an interface that has been stable for a decade and is in use in any number of scripts, so changing it in an incompatible way is something that ought to be considered only for very significant reasons.

The documentation (e.g. help text) is evidently not clear enough though - perhaps we can improve that; any suggestions welcome!

Vulcanostrol commented 3 years ago

@cannam ah, that makes sense. And yes, I do agree then that the -R option is worth having. I just think that the help text (Select realtime mode (implies --no-threads)) might not be clear enough for some users, so I might do a pull request with improved documentation.

Thanks for the clarification!

edit: on the topic of changing the library in a way that is incompatible while it is being used by many people is a very good point, one that an absolute junior like me does not think about often enough ;)