[Spice-devel] [PATCH spice-streaming-agent 1/1] Adding gstreamer based plugin
Snir Sheriber
ssheribe at redhat.com
Sun Mar 4 10:09:50 UTC 2018
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 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)
>
> 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
More information about the Spice-devel
mailing list