NMOS GStreamer sender plugin #17
Conversation
All functions translated and created new json test files
NMOS is only initialized on Playing state of plugin NMOS is only initialized on Playing state of plugin
| @@ -5,7 +5,7 @@ | |||
| "label": "BISECT OSSRF Node", | |||
There was a problem hiding this comment.
Avoid committing changes (in particular, configuration files) when those changes are not material, that is, only some values have been changed for testing purposes.
| @@ -0,0 +1,72 @@ | |||
| { | |||
There was a problem hiding this comment.
Do we need this file? Is it an example that is useful? If yes, then we should use a name that is more descriptive.
|
|
||
| switch(property_id) | ||
| { | ||
| case 1: |
There was a problem hiding this comment.
Can we have constants for this, so that it is harder to cause problems if we add more properties?
This makes me think of something else: I'd like those constants to be constexpr but this file is .c. Can we make it a .cpp and declare that needs external linkage as extern "C"?
That way we wouldn't be limited to develop in C.
There was a problem hiding this comment.
I was checking this out, and I noticed that the property_id is something required to assign to any property on moment of creation. Since get and set property functions are called internally by GStreamer, it needs to have specifically those entry arguments making it so if there was to implement another identifier for the switch case we would before need to translate those int's to the created enums. Is that what I'm supposed to do?
There was a problem hiding this comment.
Yes. A static_cast<> is appropriate there.
| } | ||
|
|
||
| #define VERSION "0.1" | ||
| #define PACKAGE "gst-plugins-ugly" |
There was a problem hiding this comment.
These need to be changed.
PACKAGE should be the library name.
PACKAGE_NAME should be AMWA NMOS Sender and Receiver Framework Plugins
GST_PACKAGE_ORIGIN: https://www.amwa.tv/
GST_PACKAGE_LICENSE: Apache-2.0
LufeBisect
force-pushed
the
nmos-plugin-creation
branch
3 times, most recently
from
719d586 to
dadc1d4
Compare
LufeBisect
force-pushed
the
nmos-plugin-creation
branch
3 times, most recently
from
9c38482 to
eccee4d
Compare
|
Applied fixes and rearranged code to not need translation library. |
| nlohmann::json create_device_config(const ConfigFields& config); | ||
| nlohmann::json create_sender_config(const ConfigFields& config); | ||
|
|
||
| const std::string get_node_id(const char* node_configuration_location); |
There was a problem hiding this comment.
const in a return by copy is useless. Please remove all of these.
It would make sense if you were returning a reference, e.g. const std::string&.
|
|
||
| const std::string get_node_id(const char* node_configuration_location) | ||
| { | ||
|
|
There was a problem hiding this comment.
Please remove the empty lines at the start (or end) of a block.
| using namespace bisect; | ||
| using json = nlohmann::json; | ||
|
|
||
| ConfigFields create_default_config_fields() |
There was a problem hiding this comment.
We've been using the following conventions:
types: this_is_a_type_t, in snake-case and with a _t suffix.
member variables (of a class): this_is_a_member_variable_, with a _ suffix.
member variables (of a struct, when it's just a "bag-of-data"): this_is_a_member_variable, with no suffix.
LufeBisect
force-pushed
the
nmos-plugin-creation
branch
from
eccee4d to
8e9ce32
Compare
|
Just fixed all issues that were talked, when possible please review my code once again. |
LufeBisect
force-pushed
the
nmos-plugin-creation
branch
4 times, most recently
from
a653739 to
8a6cf99
Compare
LufeBisect
force-pushed
the
nmos-plugin-creation
branch
from
8a6cf99 to
d33931d
Compare
Description
In this pull request, I have developed a GStreamer plugin with NMOS integration, that uses internal element linking so that it's able to use rtpvrawpay and udpsink, and still being able to process the required info for NMOS communication.
For this purpose I also had to create a middleman file so that the GStreamer C code could communicate with the CPP api and integrate it as part of it's routine. Beyond that, I have also defined the required plugin functionalities such as caps negotiation, being able to set properties and being able to change whats being used depending on the state of the pipeline.
Usage Example:
After compiling the plugin, you either need to copy the file
/build/Release/plugins/libgstnmossender.soand add it to the gstreamer plugins folder or set your plugin path to the project path.Sending an NMOS Stream with UYVP Encoding:
gst-launch-1.0 videotestsrc ! videoconvert ! "video/x-raw, format=UYVP, width=640, height=480, framerate=50/1" ! nmossender destination-address="192.168.88.92" destination-port=5004Notes
For now, some parts are still configured by using the JSON files as the source of the info for the NMOS configuration, that still needing to be changed on a future PR.