Add getSampleRate()

[bmatherly]

This is a convenience function to allow applications to easily know what sample rate the stretcher was initalized with to check if it needs to be recreated.

[cannam]

I am in two minds about this one. The content of the commit of course looks fine, but I'm not convinced that it is worth adding to the API for.

The stretcher has two sorts of construction parameters - fixed ones like sample rate and channel count, and variable ones like ratios and options. At the moment you can query the channel count after construction (even though it never changes), but not the sample rate, so there is undeniably an inconsistency there. However, I'm not entirely sure it is a good thing to be able to query the channel count either, and I slightly regret including that method already. I think the stretcher object should be contained (in application code) in a context that fixes these construction parameters, so its sample rate and channel count are known explicitly, rather than expecting to query these values and make a new stretcher if they don't match - which may be prone to cause hard-to-diagnose errors.

Can you persuade me otherwise?

[bmatherly]

Hi. Thank you for your thoughtful consideration.

I implemented a filter wrapper for rubberband in the MLT framework. In that framework, a filter does not know the sample rate of the audio that it will be receiving at construction time. So the filter needs to detect the rate when it receives its first audio. In some cases, for example, if the user changes the project configuration, the filter could spontaneously receive audio frames with different channel count or sampling frequency.

Here is the line of code that runs with every frame to detect the changing conditions: https://github.com/mltframework/mlt/blob/master/src/modules/rubberband/filter_rbpitch.cpp#L111 As you can see, the channel count change can be detected by querying the stretcher. But for the sampling frequency, I am forced to save a member variable to remember the initalized sampling frequency: https://github.com/mltframework/mlt/blob/master/src/modules/rubberband/filter_rbpitch.cpp#L35 The sole purpose of that member variable is to detect when the frequency changes because I can not query the stretcher.

I understand that it is not difficult or expensive to keep a member variable to remember the frequency. So this change is only a convenience in this scenario.

From my perspective, the reason to accept the patch would be: 1) Consistency: since the channel function already exists 2) Small convenience for situations like I describe above

I do not feel strongly about the patch. I only offer it because it was easy for me to produce and test in my environment. So if you reject the patch, my application will still be fine. But if you accept it, I will probably change my code to use the new function.

[luzpaz]

Any verdict on this ?