Cool!
On 22 Feb 2018, at 17:20, Snir Sheriber <ssheribe@xxxxxxxxxx> wrote:
Gstreamer based plugin utilizing gstreamer elements to capture
screen from X, convert and encode into h264 stream using x264enc
gstreamer plugin.
Configure with --with-gst, will be built as a separate plugin.
Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
configure.ac | 8 ++
src/Makefile.am | 26 +++++++
src/gst-plugin.cpp | 222 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 256 insertions(+)
create mode 100644 src/gst-plugin.cpp
diff --git a/configure.ac b/configure.ac
index 5aab662..ee2ef68 100644
--- a/configure.ac
+++ b/configure.ac
@@ -52,6 +52,13 @@ AC_SUBST(JPEG_LIBS)
AC_CHECK_HEADER([catch/catch.hpp],,[AC_MSG_ERROR([Could not find Catch dependency header (catch/catch.hpp)])])
+AC_ARG_WITH([gst], AS_HELP_STRING([--with-gst], [Build with the gstreamer plugin]))
+have_gst=no
+if test "x$with_gst" = "xyes"; then
+ PKG_CHECK_MODULES(GST, [gstreamer-1.0 gstreamer-app-1.0], [have_gst=yes], [have_gst=no])
+fi
+AM_CONDITIONAL([HAVE_GST],[test "$have_gst" = "yes"])
+
dnl ===========================================================================
dnl check compiler flags
@@ -102,6 +109,7 @@ AC_MSG_NOTICE([
prefix: ${prefix}
C compiler: ${CC}
C++ compiler: ${CXX}
+ Gstreamer plugin: ${have_gst}
Now type 'make' to build $PACKAGE
])
diff --git a/src/Makefile.am b/src/Makefile.am
index 3717b5c..c09a2d7 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -5,6 +5,8 @@
NULL =
SUBDIRS = . unittests
+plugin_LTLIBRARIES =
+plugindir = $(pkglibdir)/plugins
AM_CPPFLAGS = \
-DSPICE_STREAMING_AGENT_PROGRAM \
@@ -56,3 +58,27 @@ spice_streaming_agent_SOURCES = \
jpeg.cpp \
jpeg.hpp \
$(NULL)
+
+if HAVE_GST
+plugin_LTLIBRARIES += gst-plugin.la
+
+gst_plugin_la_LDFLAGS = \
+ -module -avoid-version \
+ $(RELRO_LDFLAGS) \
+ $(NO_INDIRECT_LDFLAGS) \
+ $(NULL)
+
+gst_plugin_la_LIBADD = \
+ $(GST_LIBS) \
+ $(NULL)
+
+gst_plugin_la_SOURCES = \
+ gst-plugin.cpp \
+ $(NULL)
+
+gst_plugin_la_CPPFLAGS = \
+ -I$(top_srcdir)/include \
+ $(SPICE_PROTOCOL_CFLAGS) \
+ $(GST_CFLAGS) \
+ $(NULL)
+endif
diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
new file mode 100644
index 0000000..2d170a0
--- /dev/null
+++ b/src/gst-plugin.cpp
@@ -0,0 +1,222 @@
+#include <config.h>
+#include <cstring>
+#include <exception>
+#include <stdexcept>
+#include <sstream>
+#include <memory>
+#include <syslog.h>
+#include <unistd.h>
+#include <gst/gst.h>
+#include <gst/app/gstappsink.h>
+
+#include <spice-streaming-agent/plugin.hpp>
+#include <spice-streaming-agent/frame-capture.hpp>
+
+using namespace spice::streaming_agent;
+
+namespace {
Why anonymous namespace and not namespace spice::streaming_agent? Or better yet, spice::streaming_agent::gstreamer_plugin?
+struct GstSettings
+{
+ int fps;
+ int encode_speed;
+};
+
+class GstFrameCapture final: public FrameCapture
+{
+public:
+ GstFrameCapture(const GstSettings &settings);
+ ~GstFrameCapture();
+ FrameInfo CaptureFrame() override;
+ void Reset() override;
+ SpiceVideoCodecType VideoCodecType() const override {
+ return SPICE_VIDEO_CODEC_TYPE_H264;
+ }
+private:
+ void free_buffer();
+
+ GstElement *pipeline, *capture, *sink;
+ GstSettings settings;
+ bool is_first = true;
+ int w, h;
For both readability and alignment reasons, I’d put all the Gst things together, and then int w,h then bool is_first.
+ GstSample *sample = nullptr;
+ GstBuffer *buffer = nullptr;
+ GstMapInfo map = {};
Any reason you initialize some fields and not others?
+};
+
+class GstreamerPlugin final: public Plugin
+{
+public:
+ FrameCapture *CreateCapture() override;
+ unsigned Rank() override;
+ void ParseOptions(const ConfigureOption *options);
+ SpiceVideoCodecType VideoCodecType() const override {
+ return SPICE_VIDEO_CODEC_TYPE_H264;
+ }
+private:
+ GstSettings settings = { 25, 1 };
Please use attributes here for readability.
+};
+}
+
+GstFrameCapture::GstFrameCapture(const GstSettings& settings):
+ settings(settings)
+{
+ GstElement *encoder, *convert;
+ GstCaps *caps;
+
+ pipeline = gst_pipeline_new("pipeline");
+ capture = gst_element_factory_make("ximagesrc", "capture");
+ convert = gst_element_factory_make("videoconvert", "convert");
+ encoder = gst_element_factory_make("x264enc", "encoder"); //TODO: move to use encodebin and profiles - much more generic
For testing purpose, what about a simple getenv, with default to “x264enc”?
Why not make it part of GstSettings?
+ throw std::runtime_error("One or more gstreamr element cannot be created”);
…
+ }
+
+ syslog(LOG_INFO, "fps = %d\n", settings.fps);
+ std::string str = "video/x-h264,stream-format=(string)byte-stream,framerate=" + std::to_string(settings.fps) + "/1”;
Wondering how this will generalize to other encoders… Good enough for a first pass.
+ caps = gst_caps_from_string(str.c_str());
+ g_object_set(sink,
+ "caps", caps,
+ "sync", FALSE,
+ "drop", TRUE,
+ "max-buffers", 1,
+ NULL);
+ gst_caps_unref(caps);
+
+ g_object_set(capture,
+ "use-damage", 0,
+ NULL);
+
+ gst_util_set_object_arg(G_OBJECT(encoder), "tune", "zerolatency");//stillimage,fastdecode,zerolatency
+ g_object_set(encoder,
+ "bframes", 0,
+ "speed-preset", 1, //1-ultrafast,6-med, 9-veryslow
+ NULL);
+
+ gst_bin_add_many(GST_BIN(pipeline), capture, convert, encoder, sink, NULL);
+
+ caps = gst_caps_from_string("video/x-raw,format=(string)I420");
+ if (gst_element_link(capture, convert) &&
+ gst_element_link_filtered(convert, encoder, caps) &&
+ gst_element_link(encoder, sink)) {
+ syslog(LOG_INFO, "Elements were linked successfully\n");
+ } else {
+ throw std::runtime_error("Link failed”);
…
+ }
+
+ gst_caps_unref(caps);
+ gst_element_set_state(pipeline, GST_STATE_PLAYING);//TODO: Not sure playing state is ideal for this case (timing wise)
+}
+
+void GstFrameCapture::free_buffer()
+{
+ if (buffer) {
+ gst_buffer_unmap(buffer, &map);
+ // don't unref buffer, will be unref when sample is unref
That comment seems a bit strange given the if(sample) below…
+ buffer = nullptr;
+ }
+ if (sample) {
+ gst_sample_unref(sample);
+ sample = nullptr;
+ }
+}
+
+GstFrameCapture::~GstFrameCapture()
+{
+ free_buffer();
This suggest you may want a Buffer subclass encapsulating gst_sample_get_buffer, map, unmap, etc. (follow-up).
+ gst_element_set_state(pipeline, GST_STATE_NULL);
+ gst_object_unref(pipeline);
+}
+
+void GstFrameCapture::Reset()
+{
+ //TODO
+}
+
+FrameInfo GstFrameCapture::CaptureFrame()
+{
+ free_buffer();
+
+ sample = gst_app_sink_pull_sample(GST_APP_SINK(sink));//block, timeout needed?
+
+ FrameInfo info{};
+ if (sample) {
+ buffer = gst_sample_get_buffer(sample);
+ gst_buffer_map(buffer, &map, GST_MAP_READ);
+
+ info.stream_start = is_first;
+ if (is_first) {
+ int sx,sy,ex,ey;
+ g_object_get(capture,
+ "endx", &ex,
+ "endy", &ey,
+ "startx", &sx ,
+ "starty", &sy,
+ NULL);
+ w = ex - sx;
+ h = ey - sy;
+ syslog(LOG_INFO, "%d x %d \n", w, h);
+ if ( h <= 0 || w <= 0 ) { // <=16?
+ throw std::runtime_error("Invalid stream size");
+ }
+ is_first = false;
+ }
+ info.size.width = w;
+ info.size.height = h;
+ info.buffer = map.data;
+ info.buffer_size = map.size;
+ } else {
+ syslog(LOG_ERR, "No sample- EOS or state change\n");
+ }
+
+ return info;
+}
+
+FrameCapture *GstreamerPlugin::CreateCapture()
+{
+ return new GstFrameCapture(settings);
+}
+
+unsigned GstreamerPlugin::Rank()
+{
+ return SoftwareMin;
+}
+
+void GstreamerPlugin::ParseOptions(const ConfigureOption *options)
+{
+#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);
+
+ for (; options->name; ++options) {
+ const char *name = options->name;
+ const char *value = options->value;
+
+ if (strcmp(name, "framerate") == 0) {
+ int val = atoi(value);
+ if (val > 0) {
+ settings.fps = val;
+ }
+ else {
+ arg_error("wrong framerate arg %s\n", value);
+ }
+ }
+ }
+}
+
+__attribute__ ((visibility ("default")))
+bool spice_streaming_agent_plugin_init(Agent* agent)
+{
+ if (agent->Version() != PluginVersion) {
+ return false;
+ }
+
+ gst_init(NULL, NULL);
+
+ std::unique_ptr<GstreamerPlugin> plugin(new GstreamerPlugin());
+
+ plugin->ParseOptions(agent->Options());
+
+ agent->Register(*plugin.release());
+
+ return true;
+}
--
2.14.3
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel