[Spice-devel] [PATCH spice-streaming-agent 4/4] gst-plugin: reduce number of templates being used
Snir Sheriber
ssheribe at redhat.com
Sun Aug 4 07:53:39 UTC 2019
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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel
More information about the Spice-devel
mailing list