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

[Frediano Ziglio]

> 
> > 
> > ---
> > 
> > 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)


> > ---
> >  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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel