Closed pabs3 closed 1 year ago
github-actions[bot]
commented
3 years ago Hello @pabs3! :wave: we're sorry you found a bug... so first of all, thank you very much for reporting it.
To know about progress, check in Triage. All issues are considered Backlog Candidates until work priorities align and the issue is selected for development. It will then become part of our official Backlog.
j1elo
commented
3 years ago Hi there! yeah I read the announcement yesterday, and was thinking on all the old cruft and stale code that is part of the basis of Kurento... and that I'd love to get rid of :)
To be honest, the main uses of the server are nowadays inside a Docker container, which means we haven't found too much motivation to work on older parts, as long as everything works fine on Ubuntu 16.04 / 18.04 containers. Anyway I'll tag @micaelgallego so he can chime in here and have this issue on the radar.
@pabs3 when you say
the current official versions of jsoncpp support in64, but in a different way to kmsjsoncpp
What does it mean "in a different way"? Would it break Kurento if the old fork was replaced with the new library?
If YES: would that be a binary compatibility breakage? or we're talking here about having to change source code of the library user (KMS)?
pabs3
commented
3 years ago I didn't look into the int64 stuff too closely, but I noted that JSON_HAS_INT64 is in the upstream jsoncpp but int64Value is not, but is in kmsjsoncpp, so the kms-core build fails when using upstream jsoncpp. Looking at the code, kms-core doesn't mention int64Value, so it looks like the module generator will need to change for jsoncpp to be used. This probably means a compatibility break of some kind, not sure.
build/src/server/interface/generated-cpp/Stats.cpp: In member function 'virtual void kurento::Stats::Serialize(kurento::JsonSerializer&)':
build/src/server/interface/generated-cpp/Stats.cpp:48:120: error: 'int64Value' is not a member of 'Json::ValueType'
48 | if (!s.JsonValue.isMember ("timestampMillis") || !s.JsonValue["timestampMillis"].isConvertibleTo (Json::ValueType::int64Value) ) {
| ^~~~~~
-- bye, pabs
pabs3
commented
3 years ago BTW, another similar issue is gstreamer, kms-core does build with libgstreamer1.0-dev 1.18.4-2 from Debian but the tests fail with gstreamer related failures. Seems you also have a gstreamer fork?
-- bye, pabs
j1elo
commented
3 years ago All Kurento modules have a public API that is defined in a JSON file; these are named *.kmd.json ("kmd" is "kurento module description").
This KMD JSON file is then used by kurento-module-creator (a Java program) to auto-generate 2 types of source code files:
I've checked it, and the type "int64" is used in lots of numeric values in kms-core, and some others in kms-elements. I've been able to find "type": "int64" in these:
The module creator then does the translation between this "int64" and the actual type in here:
https://github.com/Kurento/kurento-module-creator/blob/master/src/main/java/org/kurento/modulecreator/codegen/function/JsonCppTypeData.java#L108-L113
so that's the point where the symbol int64Value appears during the translation stage while generating code from the JSON API files
Getting rid of our old GStreamer fork... that's a whole totally different world of pain, so it should be tackled separately. Also there are some Kurento contributor that have been working on it in the past, but I'm not sure to what point did they achieve their target. Right now, a close collaborator is also working on it, so it might be to the benefit of everybody that we wait a bit to see if they are close to doing it.
This is already well beyond offtopic, and maybe for further discussion we should move to the Kurento Mailing List.
pabs3
commented
3 years ago Thanks for the int64 info.
Good to hear about the gstreamer work.
Feel free to close this issue if needed.
I don't intend to register a Google account so I won't be able to participate in any discussion on the mailing list.
BTW, I believe my colleague @sunweaver has sent you private mail about the BBB/Kurento Debian packaging effort.
-- bye, pabs
Prerequisites
These are MANDATORY, otherwise the issue will be automatically closed.
Issue description
Kurento uses a fork of a very old version of jsoncpp that adds support for int64. It looks like the current official versions of jsoncpp support in64, but in a different way to kmsjsoncpp. It would be great if Kurento could switch to a current official version of jsoncpp instead of the fork.
Context
As part of the effort to package BigBlueButton within Debian I have been looking at and improving the existing Kurento packaging for Debian. During that process I noticed that Kurento uses a fork of a very old version of jsoncpp. I'm not yet sure if I can help with the conversion, but I will at least help test build the results.
How to reproduce?
Expected & current behavior
The kms-core build with the current official version of jsoncpp should work but fails with:
(Optional) Possible solution
Switch to the official version of jsoncpp.
Info about your environment
Building kms-core on Debian bullseye using jsoncpp 1.9.4-4 from Debian and switching kms-jsonrpc to use jsoncpp instead of kmsjsoncpp.
About Kurento Media Server
About your Application Server
About end-user clients
Run these commands