Hi,
On 6/26/19 6:24 PM, Frediano Ziglio wrote:
Signed-off-by: Snir Sheriber <ssher...@redhat.com>
Can you explain why this is better? It's not clear from the code.
Actually it is not much better, it just seems more reasonable to me
to set this properties using a dedicated function than manipulating
a string and and then convert it.
---
src/gst-plugin.cpp | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
index 9858beb..cf660eb 100644
--- a/src/gst-plugin.cpp
+++ b/src/gst-plugin.cpp
@@ -8,7 +8,6 @@
#include <cstring>
#include <exception>
#include <stdexcept>
-#include <sstream>
#include <memory>
#include <syslog.h>
#include <unistd.h>
@@ -132,34 +131,33 @@ GstElement
*GstreamerFrameCapture::get_encoder_plugin(const GstreamerEncoderSett
GList *filtered;
GstElement *encoder;
GstElementFactory *factory = nullptr;
- std::stringstream caps_ss;
switch (settings.codec) {
case SPICE_VIDEO_CODEC_TYPE_H264:
- caps_ss << "video/x-h264" << ",stream-format=(string)byte-stream";
+ sink_caps.reset(gst_caps_new_simple("video/x-h264", "stream-format",
G_TYPE_STRING, "byte-stream", nullptr));
break;
case SPICE_VIDEO_CODEC_TYPE_MJPEG:
- caps_ss << "image/jpeg";
+ sink_caps.reset(gst_caps_new_empty_simple("image/jpeg"));
break;
case SPICE_VIDEO_CODEC_TYPE_VP8:
- caps_ss << "video/x-vp8";
+ sink_caps.reset(gst_caps_new_empty_simple("video/x-vp8"));
break;
case SPICE_VIDEO_CODEC_TYPE_VP9:
- caps_ss << "video/x-vp9";
+ sink_caps.reset(gst_caps_new_empty_simple("video/x-vp9"));
break;
case SPICE_VIDEO_CODEC_TYPE_H265:
- caps_ss << "video/x-h265";
+ sink_caps.reset(gst_caps_new_empty_simple("video/x-h265"));
break;
default : /* Should not happen - just to avoid compiler's complaint */
throw std::logic_error("Unknown codec");
}
- caps_ss << ",framerate=" << settings.fps << "/1";
+ gst_caps_set_simple(sink_caps.get(), "framerate", GST_TYPE_FRACTION,
settings.fps, 1, nullptr);
+ gchar *caps_str = gst_caps_to_string(sink_caps.get());
As we are using exception I would avoid using naked pointers.
Will send another proposal
Snir.
encoders =
gst_element_factory_list_get_elements(GST_ELEMENT_FACTORY_TYPE_VIDEO_ENCODER,
GST_RANK_NONE);
- sink_caps.reset(gst_caps_from_string(caps_ss.str().c_str()));
filtered = gst_element_factory_list_filter(encoders, sink_caps.get(),
GST_PAD_SRC, false);
if (filtered) {
- gst_syslog(LOG_NOTICE, "Looking for encoder plugins which can
produce a '%s' stream", caps_ss.str().c_str());
+ gst_syslog(LOG_NOTICE, "Looking for encoder plugins which can
produce a '%s' stream", caps_str);
for (GList *l = filtered; l != nullptr; l = l->next) {
if (!factory &&
!settings.encoder.compare(GST_ELEMENT_NAME(l->data))) {
factory = (GstElementFactory*)l->data;
@@ -169,14 +167,15 @@ GstElement
*GstreamerFrameCapture::get_encoder_plugin(const GstreamerEncoderSett
if (!factory && !settings.encoder.empty()) {
gst_syslog(LOG_WARNING,
"Specified encoder named '%s' cannot produce '%s'
stream, make sure matching gst.codec is specified
and plugin's availability",
- settings.encoder.c_str(), caps_ss.str().c_str());
+ settings.encoder.c_str(), caps_str);
}
factory = factory ? factory : (GstElementFactory*)filtered->data;
gst_syslog(LOG_NOTICE, "'%s' encoder plugin is used",
GST_ELEMENT_NAME(factory));
} else {
- gst_syslog(LOG_ERR, "No suitable encoder was found for '%s'",
caps_ss.str().c_str());
+ gst_syslog(LOG_ERR, "No suitable encoder was found for '%s'",
caps_str);
}
+ g_free(caps_str);
This line should not exist.
encoder = factory ? gst_element_factory_create(factory, "encoder") :
nullptr;
if (encoder) { // Invalid properties will be ignored silently
@@ -351,10 +350,12 @@ void GstreamerFrameCapture::xlib_capture()
throw std::runtime_error("Failed to wrap image in gstreamer
buffer");
}
- std::stringstream ss;
-
- ss << "video/x-raw,format=BGRx,width=" << image->width << ",height=" <<
image->height << ",framerate=" << settings.fps << "/1";
- GstCapsUPtr caps(gst_caps_from_string(ss.str().c_str()));
+ GstCapsUPtr caps(gst_caps_new_simple("video/x-raw",
+ "format", G_TYPE_STRING, "BGRx",
+ "width", G_TYPE_INT, image->width,
+ "height", G_TYPE_INT,
image->height,
+ "framerate", GST_TYPE_FRACTION,
settings.fps, 1,
+ nullptr));
// Push sample
GstSampleUPtr appsrc_sample(gst_sample_new(buf, caps.get(), nullptr,
nullptr));
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
