Re: [PATCH spice-streaming-agent 4/4] gst-plugin: reduce number of templates being used

Snir Sheriber <ssheribe@xxxxxxxxxx> · Sun, 4 Aug 2019 10:53:39 +0300

On 8/1/19 7:17 PM, Frediano Ziglio wrote:
---

This patch is not really necessary, just a suggestion, it's a bit hacky
but would save some code.
Other options would be to use explicit template specialization or to
leave it as is.

Sure, what I don't like is that is surely not type safe, you can instantiate
a GstMiniObjectUPtr of whatever, even an "int" type and compiler won't
complain at all, witch is a good thing of C++.
I'm thinking possible changes to this patch like traits and/or macros to
declare allowed types.
Certain the type is getting a bit long ("GstMiniObjectUPtr<GstSample>"),
but this could be solve by typedefs (well, this was solved by using lines).

What about:


template <typename T>
struct is_gst_mini_type {
};

template <typename T, typename = typename is_gst_mini_type<T>::type>
struct GstMiniObjectDeleter {
     void operator()(T* p)
     {
         gst_mini_object_unref(GST_MINI_OBJECT_CAST(p));
     }
};

template <typename T>
using GstMiniObjectUPtr = std::unique_ptr<T, GstMiniObjectDeleter<T>>;

#define DECLARE_GST_MINI_TYPE(name) \
template <> struct is_gst_mini_type<name> { \
         typedef name *type; \
}; \
using name ## UPtr = GstMiniObjectUPtr<name>;

DECLARE_GST_MINI_TYPE(GstSample)


Actually also the GstObjectUPtr is not really type safe

I'm not sure i wouldn't prefer to just do something like this for
simplicity:

template <typename T>
struct GstDeleter {
    void operator()(T* p)
    {
        gst_object_unref(p);
    }
};

template <>
struct GstDeleter <GstCaps> {
    void operator()(GstCaps* p)
    {
        gst_caps_unref(p);
    }
};

template <>
struct GstDeleter <GstSample> {
    void operator()(GstSample* p)
    {
        gst_sample_unref(p);
    }
};

template <>
struct GstDeleter <GstBuffer> {
    void operator()(GstBuffer* p)
    {
        gst_buffer_unref(p);
    }
};

template <typename T>
using GstUPtr = std::unique_ptr<T, GstDeleter<T>>;

Snir.

---
  src/gst-plugin.cpp | 44 ++++++++++++++------------------------------
  1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
index c7412c5..5f4cc3d 100644
--- a/src/gst-plugin.cpp
+++ b/src/gst-plugin.cpp
@@ -49,32 +49,16 @@ struct GstObjectDeleter {
  template <typename T>
  using GstObjectUPtr = std::unique_ptr<T, GstObjectDeleter<T>>;
  
-struct GstCapsDeleter {
-    void operator()(GstCaps* p)
-    {
-        gst_caps_unref(p);
-    }
-};
-
-using GstCapsUPtr = std::unique_ptr<GstCaps, GstCapsDeleter>;
-
-struct GstSampleDeleter {
-    void operator()(GstSample* p)
-    {
-        gst_sample_unref(p);
-    }
-};
-
-using GstSampleUPtr = std::unique_ptr<GstSample, GstSampleDeleter>;
-
-struct GstBufferDeleter {
-    void operator()(GstBuffer* p)
+template <typename T>
+struct GstMiniObjectDeleter {
+    void operator()(T* p)
      {
-        gst_buffer_unref(p);
+        gst_mini_object_unref(GST_MINI_OBJECT_CAST(p));
      }
  };
  
-using GstBufferUPtr = std::unique_ptr<GstBuffer, GstBufferDeleter>;
+template <typename T>
+using GstMiniObjectUPtr = std::unique_ptr<T, GstMiniObjectDeleter<T>>;
  
  class GstreamerFrameCapture final : public FrameCapture
  {
@@ -89,7 +73,7 @@ public:
      std::vector<DeviceDisplayInfo> get_device_display_info() const
      override;
  private:
      void free_sample();
-    GstElement *get_encoder_plugin(const GstreamerEncoderSettings
&settings,
GstCapsUPtr &sink_caps);
+    GstElement *get_encoder_plugin(const GstreamerEncoderSettings
&settings,
GstMiniObjectUPtr<GstCaps> &sink_caps);
      GstElement *get_capture_plugin(const GstreamerEncoderSettings
      &settings);
      void pipeline_init(const GstreamerEncoderSettings &settings);
      Display *const dpy;
@@ -97,7 +81,7 @@ private:
      void xlib_capture();
  #endif
      GstObjectUPtr<GstElement> pipeline, capture, sink;
-    GstSampleUPtr sample;
+    GstMiniObjectUPtr<GstSample> sample;
      GstMapInfo map = {};
      uint32_t last_width = ~0u, last_height = ~0u;
      uint32_t cur_width = 0, cur_height = 0;
@@ -134,7 +118,7 @@ GstElement
*GstreamerFrameCapture::get_capture_plugin(const GstreamerEncoderSett
  }
  
  GstElement *GstreamerFrameCapture::get_encoder_plugin(const
  GstreamerEncoderSettings &settings,
-                                                      GstCapsUPtr
&sink_caps)
+
GstMiniObjectUPtr<GstCaps>
&sink_caps)
  {
      GList *encoders;
      GList *filtered;
@@ -238,7 +222,7 @@ void GstreamerFrameCapture::pipeline_init(const
GstreamerEncoderSettings &settin
      if (!convert) {
          throw std::runtime_error("Gstreamer's 'autovideoconvert' element
          cannot be created");
      }
-    GstCapsUPtr sink_caps;
+    GstMiniObjectUPtr<GstCaps> sink_caps;
      GstObjectUPtr<GstElement> encoder(get_encoder_plugin(settings,
      sink_caps));
      if (!encoder) {
          throw std::runtime_error("Gstreamer's encoder element cannot be
          created");
@@ -260,7 +244,7 @@ void GstreamerFrameCapture::pipeline_init(const
GstreamerEncoderSettings &settin
      gst_bin_add(bin, encoder);
      gst_bin_add(bin, sink);
  
-    GstCapsUPtr caps(gst_caps_from_string("video/x-raw(ANY)"));
+    GstMiniObjectUPtr<GstCaps>
caps(gst_caps_from_string("video/x-raw(ANY)"));
      link = gst_element_link(capture.get(), convert.get()) &&
             gst_element_link_filtered(convert.get(), encoder.get(),
             caps.get()) &&
             gst_element_link_filtered(encoder.get(), sink.get(),
             sink_caps.get());
@@ -362,7 +346,7 @@ void GstreamerFrameCapture::xlib_capture()
          throw std::runtime_error("Cannot capture from X");
      }
  
-    GstBufferUPtr buf(gst_buffer_new_wrapped_full((GstMemoryFlags)0,
image->data,
+    GstMiniObjectUPtr<GstBuffer>
buf(gst_buffer_new_wrapped_full((GstMemoryFlags)0, image->data,
                                                    image->height *
                                                    image->bytes_per_line,
                                                    0,
                                                    image->height *
                                                    image->bytes_per_line,
                                                    image,
                                                    free_ximage));
@@ -370,7 +354,7 @@ void GstreamerFrameCapture::xlib_capture()
          throw std::runtime_error("Failed to wrap image in gstreamer
          buffer");
      }
  
-    GstCapsUPtr caps(gst_caps_new_simple("video/x-raw",
+    GstMiniObjectUPtr<GstCaps> 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,
@@ -378,7 +362,7 @@ void GstreamerFrameCapture::xlib_capture()
                                           nullptr));
  
      // Push sample
-    GstSampleUPtr appsrc_sample(gst_sample_new(buf.get(), caps.get(),
nullptr, nullptr));
+    GstMiniObjectUPtr<GstSample> appsrc_sample(gst_sample_new(buf.get(),
caps.get(), nullptr, nullptr));
      if (gst_app_src_push_sample(GST_APP_SRC(capture.get()),
      appsrc_sample.get()) != GST_FLOW_OK) {
          throw std::runtime_error("gstramer appsrc element cannot push
          sample");
      }
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel







