HI,
On 8/1/19 6:27 PM, Kevin Pouget wrote:
Hello Snir,
On Thu, Aug 1, 2019 at 5:02 PM Snir Sheriber <ssheribe@xxxxxxxxxx> wrote:
Hi,
On 8/1/19 3:43 PM, Kevin Pouget wrote:
On Wed, Jul 31, 2019 at 12:09 PM Kevin Pouget <kpouget@xxxxxxxxxx> wrote:
On Wed, Jul 31, 2019 at 11:50 AM Frediano Ziglio <fziglio@xxxxxxxxxx> 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 ... ,
I am confused with this: how should the 2nd vp8 encoder get into execution?
at the moment the server requests "vp8" playback, so a second gst.vp8
plugin cannot be selected
or you have something else in mind?
For the switching codec use case it is indeed doesn't matter
It's just make more sense to me to associate the properties in
such manner since the available properties are usually associated
with the encoder-plugin and not with the codec type itself
Another point is that may also be used for future purposes.
For example to register h264 encoder in two different
configurations and switch between them, or to register hw
encoder and sw encoder as a fallback with same codec...
Snir.
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel