thestk / rtaudio

A set of C++ classes that provide a common API for realtime audio input/output across Linux (native ALSA, JACK, PulseAudio and OSS), Macintosh OS X (CoreAudio and JACK), and Windows (DirectSound, ASIO, and WASAPI) operating systems.
Other
1.49k stars 318 forks source link

Version 6.0 breaks compatibility for no reason. #365

Closed AlexandreRouma closed 1 year ago

AlexandreRouma commented 2 years ago

Hello,

In 6.0, RtAudioError has been renamed RtAudioErrorType. This breaks any code written for 5.2 and lower. Would it be possible to add a typedef RtAudioErrorType RtAudioError in order to keep comptability with older code? Either that or at least have an easy way to tell the versions appart with macros to make sure anything compiled for 6.0+has the correct type name.

bkerler commented 2 years ago

Additionally to Alexandre's comment, I'd like to add that in the current code, the probed bool on the DeviceInfo struct is missing which is also breaking compatibility with current projects and the python code / examples as well. For backward compatibility it would be nice to have the same functionality but with a warning considering code being outdated so that people are aware.

garyscavone commented 2 years ago

I'm open to proposed updates to address these issues. However, the recent API changes are so significant that I think it is better that people become immediately aware of them (rather than have previous code compile but potentially not work). I'm also a bit hesitant to have a gazillion version checks scattered amongst the over 11,000 lines of code in RtAudio.cpp, thus making it harder to understand. I would point out that RtAudioError in the previous versions was an exception handling class, while in the newer version it is simply an enum, so I don't see how the proposed typedef would maintain compatibility with older code.

AlexandreRouma commented 2 years ago

Well, the version check wouldn't be in rtaudio. What I'm suggesting would be a clearly defined way for code using RtAudio to tell version 6.0+ version apart from lower versions.

I wasn't sure the difference between RtAudioError and RtAudioErrorType, haven't really checked the library code in detail, just know that it's what breaks the code. (assumed it was just a rename as the names are so close).

I use RtAudio in SDR++ which has several tens of thousands of users all on different OSes with different versions of RtAudio. Lately this change has started breaking it for anyone running 6.0 or later. With no way to precisely know what version of RtAudio is installed on a system, I'd have to stop supporting one group of user or the other which is not acceptable. I also can't expect the 6.0 users to downgrade or 5.2 users to upgrade because that would break other software on the distro.

Having so many breaking changes is extremely problematic when you can't tell in code what version of the library the users have. Some distros will have 5.2 for years to come, some will update to 6.0, there's currently no way to tell.

lp35 commented 2 years ago

Worth mentioning that the new API is also breaking silently the functioning of the device lookup. It's why I would recommend to change the type of getDeviceInfo() type, or maybe change the signature of the method?

unsigned int devCnt = iHnd->getDeviceCount();
for (unsigned int k = 0; k < devCnt; k++ ) 
{
    RtAudio::DeviceInfo info = iHnd->getDeviceInfo(k);
    ///...
}
lp35 commented 2 years ago

Well, the version check wouldn't be in rtaudio. What I'm suggesting would be a clearly defined way for code using RtAudio to tell version 6.0+ version apart from lower versions.

I wasn't sure the difference between RtAudioError and RtAudioErrorType, haven't really checked the library code in detail, just know that it's what breaks the code. (assumed it was just a rename as the names are so close).

I use RtAudio in SDR++ which has several tens of thousands of users all on different OSes with different versions of RtAudio. Lately this change has started breaking it for anyone running 6.0 or later. With no way to precisely know what version of RtAudio is installed on a system, I'd have to stop supporting one group of user or the other which is not acceptable. I also can't expect the 6.0 users to downgrade or 5.2 users to upgrade because that would break other software on the distro.

Having so many breaking changes is extremely problematic when you can't tell in code what version of the library the users have. Some distros will have 5.2 for years to come, some will update to 6.0, there's currently no way to tell.

I think there is a choice to do here. RTAudio is designed to be easily drop-able in a project without compiling a side library. Either it's a kind of "header+ one cpp" library that is embed with the application, and you don't need to ensure retro-compatibility, or you make it as a real, standalone lib with clear versioning scheme and try to prevent any breaking change on the API.

garyscavone commented 2 years ago

I'm afraid I don't have the time to attempt to maintain retro-compatability. It was never meant to be a standalone library, though people obviously use it that way.

apiel commented 1 year ago

Does it mean that you would recommend to use it by include it directly in our project and not use librtaudio6?

garyscavone commented 1 year ago

There is the RtAudio::getVersion() function to determine the version you are working with. At this point, version 6.0 has not even been officially released from beta, though I hope to do that in the coming week(s).