search

mozilla / janus-plugin-rs

Rust crate for wrapping the Janus C plugin API, so you can build Janus plugins in Rust.
Mozilla Public License 2.0
46 stars 18 forks source link

JANUS_PLUGIN_API_VERSION without refcount feature #8

Closed ivanovaleksey closed 6 years ago

ivanovaleksey commented 6 years ago

Hello @mquander,

I have faced with a little problem with my EchoTest plugin. Today I rebuild a Docker image and apparently Janus Gateway source code was updated. As it turns out JANUS_PLUGIN_API_VERSION was bumped from 8 to 9.

Currently I use 0.7.1 version without refcount feature. In that case JANUS_PLUGIN_API_VERSION = 8.

I got the following error.

[ERR] [janus.c:main:3912] The 'janus.plugin.echotest' plugin was compiled against an older version of the API (8 < 9), skipping it: update it to enable it again

It is not a problem really since the plugin is demo-only and could be easily build using refcount. I would like to ask you for advice. Janus Gateway already defined new 9 API version on master not refcount branch. And the crate expects that 9 could be only on refcount branch. Should we change it somehow?

mqp commented 6 years ago

Hmm. I think the plugin versions are not doing a good job of unambiguously indicating the features the plugin is supposed to support -- for example, this change matches a gateway change in the refcount branch, concurrently with the version being bumped to 9 in that branch, but now the master has bumped the version to 9 without including that change. So 9 means one thing in the refcount branch and another thing in master.

Regardless, it doesn't seem like this library should be opinionated about the plugin version, anyway. This library is just a set of tools for making a plugin -- you can use them to make a plugin that is compatible with this version or that version. So I'm inclined to change it to just leave it up to your plugin code to decide what your plugin's API version is. Does that sound reasonable to you?

See https://github.com/mozilla/janus-plugin-rs/pull/9.

ivanovaleksey commented 6 years ago

It would be nice to get an answer to your comment on refcount PR, whether it is just a mistake or not. It was convenient just to have JANUS_PLUGIN_API_VERSION defined while it was unambiguous. But now it looks more like a problem than a helper. Anyway, it is not a big deal to define API version in plugin's code manually.

mqp commented 6 years ago

(This is changed as suggested above in 0.8.0.)