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

[Kevin Pouget]

On Wed, Jul 31, 2019 at 11:50 AM Frediano Ziglio <fziglio at redhat.com> wrote:
>
> >
> > 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) ?

maybe the easiest is to say that "gst.XXX=YYY" applies to all the
codecs, and "gst.CODEC.XXX=YYY" is only for one codec, with
XXX=prop|encoder at the moment.