[Spice-devel] [PATCH spice-streaming-agent 1/1] Adding gstreamer based plugin
Frediano Ziglio
fziglio at redhat.com
Thu Mar 1 07:32:04 UTC 2018
>
> 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 at redhat.com>
> > > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > >
> > > > 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)
> > > > > +}
> > > > > +
> > > > > +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.
> > > > > + }
> > > > > +
> > > > > + 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
More information about the Spice-devel
mailing list