[Spice-devel] [RFC spice-streaming-agent 1/2] gst-plugin: allow the instantiation of multiple GST encoder plugins
Snir Sheriber
ssheribe at redhat.com
Thu Aug 1 15:01:33 UTC 2019
Hi,
On 8/1/19 3:43 PM, Kevin Pouget wrote:
> On Wed, Jul 31, 2019 at 12:09 PM Kevin Pouget <kpouget at redhat.com> wrote:
>> 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.
> Hello,
>
> I did the modifications below to allow passing codec-specific gst parameters:
Basically Gstreamer properties are more tied to the encoder than
the codec type e.g. vaapih264enc plugin has a property named "low-delay-b"
and x264enc hasn't.
Now I'm thinking maybe i should have allow to set properties only when the
encoder is specified :P, anyway i think we do need to parse it in order
somehow
i.e. vp8 + its encoder + its props... , another vp8 + its props ... ,
h264 + its encoder...
I did no try it yet, I'll test it and have a closer look on everything,
i like this feature!
Thanks,
Snir.
>
>> gst.codec=CODEC
>> gst.prop | gst.CODEC.prop
>> gst.encoder | gst.CODEC.prop
>> framerate | gst.CODEC.framerate
> regards,
>
> Kevin
>
> ---
> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> index 5469647..4194485 100644
> --- a/src/gst-plugin.cpp
> +++ b/src/gst-plugin.cpp
> @@ -102,7 +102,8 @@ class GstreamerPlugin final: public Plugin
> public:
> FrameCapture *CreateCapture() override;
> unsigned Rank() override;
> - void ParseOptions(const ConfigureOption *options,
> SpiceVideoCodecType codec);
> + void ParseOptions(const ConfigureOption *options,
> SpiceVideoCodecType codec,
> + const std::string codec_name);
> SpiceVideoCodecType VideoCodecType() const override {
> return settings.codec;
> }
> @@ -431,7 +432,8 @@ unsigned GstreamerPlugin::Rank()
> return SoftwareMin;
> }
>
> -void GstreamerPlugin::ParseOptions(const ConfigureOption *options,
> SpiceVideoCodecType codec)
> +void GstreamerPlugin::ParseOptions(const ConfigureOption *options,
> + SpiceVideoCodecType codec, const
> std::string codec_name)
> {
> settings.codec = codec;
>
> @@ -439,15 +441,17 @@ void GstreamerPlugin::ParseOptions(const
> ConfigureOption *options, SpiceVideoCo
> const std::string name = options->name;
> const std::string value = options->value;
>
> - if (name == "framerate") {
> + if (name == "framerate" || name == "gst." + codec_name +
> ".framerate") {
> try {
> settings.fps = std::stoi(value);
> +
> } catch (const std::exception &e) {
> - throw std::runtime_error("Invalid value '" + value +
> "' for option 'framerate'.");
> + throw std::runtime_error("Invalid value '" + value + '" "
> + "for option '" + name + "'.");
> }
> - } else if (name == "gst.encoder") {
> + } else if (name == "gst.encoder" || name == "gst." +
> codec_name +".encoder") {
> settings.encoder = value;
> - } else if (name == "gst.prop") {
> + } else if (name == "gst.prop" || name == "gst." + codec_name
> + ".prop") {
> size_t pos = value.find('=');
> if (pos == 0 || pos >= value.size() - 1) {
> gst_syslog(LOG_WARNING, "Property input is invalid
> ('%s' Ignored)", value.c_str());
> @@ -488,7 +492,7 @@ SPICE_STREAMING_AGENT_PLUGIN(agent)
> }
>
> auto plugin = std::make_shared<GstreamerPlugin>();
> - plugin->ParseOptions(agent->Options(), codec_type);
> + plugin->ParseOptions(agent->Options(), codec_type, value);
> agent->Register(plugin);
> }
> }
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list