>
> From: test <test@localhost.localdomain>
>
> This allows to set plugin key=value properties on run time.
> To add encoder plugin property use the following syntax:
> -c gst.prop="property=value" -c gst.prop="property2=value2"...
> Make sure syntax is accurate and that the property is supported by
> the chosen plugin, wrong properties may ignored silently.
>
> Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
> ---
>
> Difference from v1:
> -Variables naming
> -Using vector of pairs
> -Add warnings for wrong input
> -Set propeties after log msg
> -Fix commit msg
>
> *There is only basic input validation, assuming the user is
> accurate with the properties he sets.
> Currently the code checks property name exists for the encoder
> object, i had a version that checks also the value matches the
> property type but it seems to be over-coding for this purpose.
>
> ---
> src/gst-plugin.cpp | 28 +++++++++++++++++++++++-----
> 1 file changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 4e802f1..3270cce 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -35,6 +35,7 @@ struct GstreamerEncoderSettings
> int fps = 25;
> SpiceVideoCodecType codec = SPICE_VIDEO_CODEC_TYPE_H264;
> std::string encoder;
> + std::vector<std::pair<std::string, std::string>> prop_pairs;
> };
>
> template <typename T>
> @@ -179,11 +180,18 @@ GstElement
> *GstreamerFrameCapture::get_encoder_plugin(const GstreamerEncoderSett
> }
>
> encoder = factory ? gst_element_factory_create(factory, "encoder") :
> nullptr;
> - if (encoder) { // Invalid properties will be ignored silently
> - /* x264enc properties */
> - gst_util_set_object_arg(G_OBJECT(encoder), "tune", "zerolatency");//
> stillimage, fastdecode, zerolatency
> - gst_util_set_object_arg(G_OBJECT(encoder), "bframes", "0");
> - gst_util_set_object_arg(G_OBJECT(encoder), "speed-preset", "1");//
> 1-ultrafast, 6-med, 9-veryslow
> + if (encoder) { // Set encoder properties
> + for (int i = 0; i < settings.prop_pairs.size(); i++) {
I would use a simple
for (const auto &prop: settings.prop_pairs) {
> + const char *prop_name = settings.prop_pairs[i].first.c_str();
I would personally avoid the temp and use "prop.first.c_str()" below
or use an alias like
const auto &name = prop.first;
const auto &value = prop.second;
> + if (!g_object_class_find_property(G_OBJECT_GET_CLASS(encoder),
> prop_name)) {
> + gst_syslog(LOG_WARNING, "'%s' property was not found for
> this encoder", prop_name);
> + continue;
> + }
> + const char *prop_val = settings.prop_pairs[i].second.c_str();
> + gst_syslog(LOG_NOTICE, "Trying to set encoder property: '%s =
> %s'", prop_name, prop_val);
> + /* Invalid properties will be ignored silently */
> + gst_util_set_object_arg(G_OBJECT(encoder), prop_name, prop_val);
> + }
> }
> gst_plugin_feature_list_free(filtered);
> gst_plugin_feature_list_free(encoders);
> @@ -449,6 +457,16 @@ void GstreamerPlugin::ParseOptions(const ConfigureOption
> *options)
> }
> } else if (name == "gst.encoder") {
> settings.encoder = value;
> + } else if (name == "gst.prop") {
> + std::string prop_name, prop_val;
> + size_t pos = value.find('=');
> + if (!pos || pos >= value.size() - 1) {
I would use a
if (pos == std::string::npos || pos == 0) {
> + gst_syslog(LOG_WARNING, "Property input is invalid ('%s'
> Ignored)", value.c_str());
> + continue;
> + }
> + prop_name = value.substr(0, pos);
> + prop_val = value.substr(pos + 1);
Why not use auto and avoid separated declaration? Potentially this will avoid
constructing empty string and then changing them.
> + settings.prop_pairs.push_back(make_pair(prop_name, prop_val));
> }
> }
> }
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel