[Spice-devel] [RFC spice-streaming-agent 1/2] gst-plugin: allow the instantiation of multiple GST encoder plugins

[Frediano Ziglio]

> 
> With this patch, spice-streaming-agent can be launched with multiple
> Gstreamer video codecs enabled:
> 
> > spice-streaming-agent -c gst.codec=vp8 -c gst.codec=vp9 ...
> ---
>  src/gst-plugin.cpp | 50 ++++++++++++++++++++++++++++------------------
>  1 file changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 9858beb..6252e42 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -102,7 +102,7 @@ class GstreamerPlugin final: public Plugin
>  public:
>      FrameCapture *CreateCapture() override;
>      unsigned Rank() override;
> -    void ParseOptions(const ConfigureOption *options);
> +    void ParseOptions(const ConfigureOption *options, SpiceVideoCodecType
> codec);
>      SpiceVideoCodecType VideoCodecType() const override {
>          return settings.codec;
>      }
> @@ -419,8 +419,10 @@ unsigned GstreamerPlugin::Rank()
>      return SoftwareMin;
>  }
>  
> -void GstreamerPlugin::ParseOptions(const ConfigureOption *options)
> +void GstreamerPlugin::ParseOptions(const ConfigureOption *options,
> SpiceVideoCodecType codec)
>  {
> +    settings.codec = codec;
> +
>      for (; options->name; ++options) {
>          const std::string name = options->name;
>          const std::string value = options->value;
> @@ -431,20 +433,6 @@ void GstreamerPlugin::ParseOptions(const ConfigureOption
> *options)
>              } catch (const std::exception &e) {
>                  throw std::runtime_error("Invalid value '" + value + "' for
>                  option 'framerate'.");
>              }
> -        } else if (name == "gst.codec") {
> -            if (value == "h264") {
> -                settings.codec = SPICE_VIDEO_CODEC_TYPE_H264;
> -            } else if (value == "vp9") {
> -                settings.codec = SPICE_VIDEO_CODEC_TYPE_VP9;
> -            } else if (value == "vp8") {
> -                settings.codec = SPICE_VIDEO_CODEC_TYPE_VP8;
> -            } else if (value == "mjpeg") {
> -                settings.codec = SPICE_VIDEO_CODEC_TYPE_MJPEG;
> -            } else if (value == "h265") {
> -                settings.codec = SPICE_VIDEO_CODEC_TYPE_H265;
> -            } else {
> -                throw std::runtime_error("Invalid value '" + value + "' for
> option 'gst.codec'.");
> -            }
>          } else if (name == "gst.encoder") {
>              settings.encoder = value;
>          }
> @@ -459,11 +447,35 @@ SPICE_STREAMING_AGENT_PLUGIN(agent)
>  {
>      gst_init(nullptr, nullptr);
>  
> -    auto plugin = std::make_shared<GstreamerPlugin>();
> +    auto options = agent->Options();
> +    for (; options->name; ++options) {
> +        const std::string name = options->name;
> +        const std::string value = options->value;
>  
> -    plugin->ParseOptions(agent->Options());
> +        if (name == "gst.codec") {
> +            SpiceVideoCodecType codec_type;
> +            if (value == "mjpeg") {
> +                codec_type = SPICE_VIDEO_CODEC_TYPE_MJPEG;
> +            } else if (value == "h264") {
> +                codec_type = SPICE_VIDEO_CODEC_TYPE_H264;
> +            } else if (value == "h265") {
> +                codec_type = SPICE_VIDEO_CODEC_TYPE_H265;
> +            } else if (value == "vp8") {
> +                codec_type = SPICE_VIDEO_CODEC_TYPE_VP8;
> +            } else if (value == "vp9") {
> +                codec_type = SPICE_VIDEO_CODEC_TYPE_VP9;
> +            } else {
> +                throw std::runtime_error("Invalid value '" + value + "' for
> option 'gst.codec'.");
> +            }
> +
> +            auto plugin = std::make_shared<GstreamerPlugin>();
> +            plugin->ParseOptions(agent->Options(), codec_type);
> +            agent->Register(plugin);
> +        }
> +    }
>  
> -    agent->Register(plugin);
> +    // no default value at the moment
> +    // (used to be h264, see GstreamerEncoderSettings::codec)
>  
>      return true;
>  }

I didn't test it but it makes perfectly sense.
On the other hand I'm wondering what will happen to the other
parameters. They will be "broadcasted" to all plugins.
For "framerate" is fine but what will happen if you specify
also properties (gst.prop) and encoder (gst.encoder) ?

Frediano