libobs: Reject plugins compiled with newer libobs
Description
Check obs_module_ver and reject if it returned a larger major/minor version so that plugins build with newer libobs won't be loaded. Patch version is ignored since the API should not be modified by a new patch version.
Note that it might be better to increment LIBOBS_API_VER in the new major beta version (such as 28.0.0-beta, 29.0.0-beta) so that the plugins built with the new major beta version is not loaded by the old OBS.
Motivation and Context
When getting a plugins that is compiled with a newer libobs version in the future, such plugins should not be loaded to avoid potential crash.
This change will block users to accidentally install a new plugins while using old OBS version.
Table below shows examples how
| OBS API version | Plugin API version | Load plugin? |
|---|---|---|
| 27.2.4 | 27.2.0 | Load |
| 27.2.4 | 28.0.0 | Load (This PR is not implemented.) |
| 28.0.0 | 27.2.0 | Load |
| 28.0.0 | 28.0.0 | Load |
| 28.0.0 | 28.0.1 | Load (The patch version is ignored.) |
| 28.0.0 | 28.1.0 | Rejected (Minor version is compared.) |
| 28.0.0 | 29.0.0 | Rejected |
| 29.0.0 | 27.2.0 | Load |
| 29.0.0 | 28.0.0 | Load |
| 29.0.0 | 29.0.0 | Load |
Instead of this PR, each plugin can reject to be loaded by checking OBS's version. This approach depends plugin developers but they have more control.
bool obs_module_load(void)
{
if (obs_get_version() < (LIBOBS_API_VER & 0xFFFF0000))
return false;
// loading something
return true;
}
How Has This Been Tested?
Overwrite LIBOBS_API_VER on a 3rd paty plugin and compile it, then checked the plugin is not loaded.
#define LIBOBS_API_VER MAKE_SEMANTIC_VERSION(28, 0, 0)
OBS_DECLARE_MODULE()
Types of changes
- Tweak (non-breaking change to improve existing functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
There are some potential edge cases here like a plugin being compiled on CI against the latest patch release which was only released for one operating system (for example OBS 27.2.3 being Windows-only). So I'd suggest that this should only look at the major and minor version and ignore patch.
I think there's a couple issues with this:
- Our recent discussions only involve plugins that rely on Qt5 because of the coming Qt6 upgrade, but all plugins are caught in the crossfire now.
- Plugins that would normally work are now gated by this version check, which means plugin authors are now required to continually bump the version and make new releases that they wouldn't have to before. EDIT: Which also means users would lose access to those plugins if they upgrade OBS until those plugin updates are released.
@Fenrirthviti mentioned something about the planned plugin manager addressing this in some fashion although I don't know the details.
Partly because of the above, the draft for the plugin manager would have handled the compatibility checks as part of the plugin manifest for the plugin manager (i.e. the plugin metadata that the plugin manager uses), but that obviously wouldn't work for self-installed plugins or during a migration, so this probably needs some further discussion on the best way to handle. Maybe a combination of both approaches.
- Plugins that would normally work are now gated by this version check, which means plugin authors are now required to continually bump the version and make new releases that they wouldn't have to before. EDIT: Which also means users would lose access to those plugins if they upgrade OBS until those plugin updates are released.
I believe you got that the wrong way around. This PR blocks plugins compiled against a newer libobs version from loading, not older. So existing plugins are unaffected, but going forward, should there be another big breaking change, a plugin built for a newer obs will no longer load in an older one.
Due to minor changes in patch versions, I believe this check should be restricted to Major/Minor however.
Oh, I did have it backwards. Never mind everything I said then. I also think patch should be removed from the check though.
So I'd suggest that this should only look at the major and minor version and ignore patch.
Should only patch-version masked or maybe minor version as well?
Plugins that would normally work are now gated by this version check, which means plugin authors are now required to continually bump the version and make new releases that they wouldn't have to before. EDIT: Which also means users would lose access to those plugins if they upgrade OBS until those plugin updates are released.
This is opposite. Old plugins are not gated by this PR. The case the plugin does not work is if plugin developers bump the version, and if user still uses old OBS.
Even without this PR, plugin-load sometimes partially fails because of sizeof(struct obs_source_info), for example, when the plugin developer chooses newer libobs version than users' OBS version. This PR will strongly reject loading such plugins entirely.
This PR does not solve the issue on Qt5 vs. Qt6 unless back-ported to the release/27.2 branch. This PR will avoid future issues similar to the Qt5 vs. Qt6.
If plugin author want to define OBS version more complexly, they can call obs_get_version() or obs_get_version_string() from obs_module_load and return failure, for example.
I would keep minor.
Updated to ignore patch version of the plugin.
Need to increment LIBOBS_API_VER in the new major beta version (such as 28.0.0-beta, 29.0.0-beta) so that the plugins built with the new major beta version is not loaded by the old OBS.
The versioning in obs.h is necessary to be considered so that I put this PR draft, though the code itself is ready.
Need to increment LIBOBS_API_VER in the new major beta version (such as 28.0.0-beta, 29.0.0-beta) so that the plugins built with the new major beta version is not loaded by the old OBS.
The versioning in
obs.his necessary to be considered so that I put this PR draft, though the code itself is ready.
Is this still necessary? What needs to be done to get this PR ready to be merged?
The current plugin template specifies the version of libobs to compile against. I think this change is ok to be merged to obs-studio.
The current plugin template specifies the version of libobs to compile against. I think this change is ok to be merged to obs-studio.
What would be the impact for plugins who are not based on the template?
What would be the impact for plugins who are not based on the template?
With this PR, if a plugin is build against a newer libobs major-minor version than the running OBS, the plugin will be rejected and a warning message will be left on the log.
Without this PR, such a plugin is usually loaded but this would be dangerous as the plugin can expect the new feature. Sometimes obs_register_source will ignore to register if the size of struct obs_source_info is bigger than that on the running OBS. Though, obs_register_source returns nothing so the plugin cannot know the source-type was registered or not.
With this PR, if a plugin is build against a newer libobs major-minor version than the running OBS, the plugin will be rejected and a warning message will be left on the log.
That does sound right to me.
@RytoEX WDYT?