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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]