>
> Hi,
>
>
> On 6/27/19 5:12 PM, Frediano Ziglio wrote:
> >> Instead of manipulating a string and convert it to caps just
> >> use dedicated functions instead
> >>
> >> Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
> >> ---
> >> Changes:
> >> -commit log
> >> -Suggesting uniqptr for caps_str
> >> --
> >> src/gst-plugin.cpp | 34 ++++++++++++++++++----------------
> >> 1 file changed, 18 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
> >> index 9858beb..0dd7796 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,35 @@ 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",
> >> + NULL));
> >> 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);
> >> + std::unique_ptr<gchar> caps_str(gst_caps_to_string(sink_caps.get()));
> >>
> > Idea it's fine. This however will call ::operator new which it's not what
> > you want. In the same file there are different deleters like
> >
> >
> > struct GstSampleDeleter {
> > void operator()(GstSample* p)
> > {
> > gst_sample_unref(p);
> > }
> > };
> >
> > using GstSampleUPtr = std::unique_ptr<GstSample, GstSampleDeleter>;
> >
> >
> > you can base your one. Glibmm uses an utility (see
> > https://gitlab.gnome.org/GNOME/glibmm/blob/b92ef23e38f9863ebe4704a916e3322eea2f1f2c/glib/glibmm/utility.h)
> > but potentially each smart pointer will contain a pointer to g_free
> > (and it's limited to g_free like a single deleter).
>
>
> How about this oneliner instead?
>
> std::unique_ptr<gchar, decltype(&g_free)>
> caps_str(gst_caps_to_string(sink_caps.get()), &g_free);
>
> Snir.
>
Maybe it will pass the 100 characters and you will need to split it in 2 lines,
but beside that I'm fine with it.
You can remove the "&" in front of the last g_free.
... omissis ...
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel