Re: [PATCH spice-streaming-agent 1/1] Adding gstreamer based plugin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, thanks for trying it!


On 03/01/2018 04:41 PM, Jonathon Jongsma wrote:
On Thu, 2018-03-01 at 02:32 -0500, Frediano Ziglio wrote:
On Wed, 2018-02-28 at 15:42 -0600, Jonathon Jongsma wrote:
On Wed, 2018-02-28 at 15:34 -0600, Jonathon Jongsma wrote:
On Thu, 2018-02-22 at 11:40 -0500, Frediano Ziglio 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>
I can say it works.
I just got around to testing this, and it seemed like it may
have
worked once, but now I can't get it to work properly. I get
debug
output like this:


spice-streaming-agent[32712]: GOT START_STOP message -- request
to
START streaming
spice-streaming-agent[32712]: streaming starts now
spice-streaming-agent[32712]: fps = 25
spice-streaming-agent[32712]: Elements were linked successfully
spice-streaming-agent[32712]: No sample- EOS or state change
spice-streaming-agent[32712]: got a frame -- size is 0 (116 ms)
(1519853449483 ms from last frame)(1519853449366907 us)
spice-streaming-agent[32712]: write_all -- 8 bytes written
spice-streaming-agent[32712]: wrote 8 bytes of header of data
msg
with
frame of size 0 bytes
spice-streaming-agent[32712]: write_all -- 0 bytes written
spice-streaming-agent[32712]: wrote data msg body of size 0
spice-streaming-agent[32712]: No sample- EOS or state change
spice-streaming-agent[32712]: got a frame -- size is 0 (0 ms)
(0 ms
from last frame)(107 us)
spice-streaming-agent[32712]: write_all -- 8 bytes written
spice-streaming-agent[32712]: wrote 8 bytes of header of data
msg
with
frame of size 0 bytes
spice-streaming-agent[32712]: write_all -- 0 bytes written
spice-streaming-agent[32712]: wrote data msg body of size 0
spice-streaming-agent[32712]: No sample- EOS or state change
spice-streaming-agent[32712]: got a frame -- size is 0 (0 ms)
(0 ms
from last frame)(50 us)
spice-streaming-agent[32712]: write_all -- 8 bytes written
spice-streaming-agent[32712]: wrote 8 bytes of header of data
msg
with
frame of size 0 bytes
spice-streaming-agent[32712]: write_all -- 0 bytes written
spice-streaming-agent[32712]: wrote data msg body of size 0
spice-streaming-agent[32712]: No sample- EOS or state change
spice-streaming-agent[32712]: got a frame -- size is 0 (0 ms)
(0 ms
from last frame)(29 us)
spice-streaming-agent[32712]: write_all -- 8 bytes written
spice-streaming-agent[32712]: wrote 8 bytes of header of data
msg
with
frame of size 0 bytes
spice-streaming-agent[32712]: write_all -- 0 bytes written
spice-streaming-agent[32712]: wrote data msg body of size 0

.....


And it seems to do this in a tight loop so you get massive
amounts
of
this output.


Hmm, after rebooting the vm, it works again. Not sure what the
issue
was. If I see it again, I'll try to dig a little bit.
After disconnecting the client and reconnecting it, I can
reproduce. It
seems like the pipeline is never transitioning to PLAYING state.


Jonathon




---
  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
I was just checking spice-server, it uses --enable-gstreamer.
Not that important.

  ==========================================================
====
==
==
=========
  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 @@
Oh, missing the copyright header.

+#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 {
+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;
+    GstSample *sample = nullptr;
+    GstBuffer *buffer = nullptr;
+    GstMapInfo map = {};
+};
+
+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 };
+};
+}
+
+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
+    sink = gst_element_factory_make("appsink", "sink");
+    if (!capture || !convert || !encoder || !sink) {
+        //TODO: check elements existence in build time
(and\or
improve error
in runtime)
+        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";
+    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)

In my failure case, the call to gst_element_set_state() returns
GST_STATE_CHANGE_ASYNC (implying that the state has not yet changed and
will happen later (in a thread or something). But I added some debug
code to listen for GST_MESSAGE_STATE_CHANGED messages on the bus, and I
never saw anything. I didn't get any farther than that.

I was not able to reproduce  (gst* 1.12.4-*), i also tried
disconcerting and reconnecting the client. Apparently it's
GST_STATE_CHANGE_ASYNC also when it works.

In order to change state into playing_state sink needs to receive
data, so it may be an issue with the ximagesrc (GST_DEBUG=
may tell something)

If the issue is indeed with the capture we can try to capture x using
xlib and push frame through appsrc.
Another thing we can do is to block after the set_state until state
will asynchronously change or to throw an err if timeout has passed
(using get_state) but it won't really solve this issue.


Snir.

+}
+
+void GstFrameCapture::free_buffer()
+{
+    if (buffer) {
+        gst_buffer_unmap(buffer, &map);
+        // don't unref buffer, will be unref when sample
is
unref
+        buffer = nullptr;
+    }
+    if (sample) {
+        gst_sample_unref(sample);
+        sample = nullptr;
+    }
+}
+
+GstFrameCapture::~GstFrameCapture()
+{
+    free_buffer();
+    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");
I think the issue is here. This just log the error and return an
empty frame which is written.
One question would be if it makes sense to have an empty frame.

Yes, that's one issue. However, I think the root cause is the pipeline
failing to transition to PLAYING state. Here we're failing to handle
that error case properly.

+    }
+
+    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;
+}
Not perfect but surely helpful.

Fine for me.

Frediano

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]