I'd add a comment saying that
The typeUPtr templates are very similar except for the unref-function.
This patch is replacing the templates with a macro which also accepts
an unref-function and is using this macro to define all typeUPtr types.
On 8/11/19 11:54 AM, Frediano Ziglio wrote:
Signed-off-by: Snir Sheriber <ssheribe@xxxxxxxxxx>
---
Another suggestion to shoten the templates + changing subject
It looks pretty neat this version, see below for minor comments.
And more type safe too.
---
src/gst-plugin.cpp | 64 +++++++++++++++-------------------------------
1 file changed, 20 insertions(+), 44 deletions(-)
diff --git a/src/gst-plugin.cpp b/src/gst-plugin.cpp
index 4d17dbc..703d5b2 100644
--- a/src/gst-plugin.cpp
+++ b/src/gst-plugin.cpp
@@ -38,43 +38,19 @@ struct GstreamerEncoderSettings
std::vector<std::pair<std::string, std::string>> prop_pairs;
};
-template <typename T>
-struct GstObjectDeleter {
- void operator()(T* p)
- {
- gst_object_unref(p);
- }
-};
-
-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)
- {
- gst_buffer_unref(p);
- }
-};
-
-using GstBufferUPtr = std::unique_ptr<GstBuffer, GstBufferDeleter>;
+#define DECLARE_UPTR(type, func) \
+struct type##Deleter { \
+ void operator()(type* p) \
+ { \
+ func(p); \
+ } \
+}; \
+using type##UPtr = std::unique_ptr<type, type##Deleter>; \
+
I have to say that I like this syntax more than the one at
https://www.spice-space.org/spice-project-coding-style-and-coding-conventions.html
and I suggested it too. On the other hand it does not conform to the style.
I'd not change the style -- too many white-space commits will
follow (although it seems less important in a macro).
(It seems this was the style of the template code before).
I would remove the last "\" at last line, the empty like will be part of the macro
which is confusing.
me too.
Not sure if it's better to indent not at first column but it's fine for me.
Are you talking about 'struct' and 'using' lines ? Yes, makes sense
to indent them to show they are part of the macro.
+DECLARE_UPTR(GstBuffer, gst_buffer_unref)
+DECLARE_UPTR(GstCaps, gst_caps_unref)
+DECLARE_UPTR(GstSample, gst_sample_unref)
+DECLARE_UPTR(GstElement, gst_object_unref)
class GstreamerFrameCapture final : public FrameCapture
{
@@ -96,7 +72,7 @@ private:
#if XLIB_CAPTURE
void xlib_capture();
#endif
- GstObjectUPtr<GstElement> pipeline, capture, sink;
+ GstElementUPtr pipeline, capture, sink;
GstSampleUPtr sample;
GstMapInfo map = {};
uint32_t last_width = ~0u, last_height = ~0u;
@@ -210,7 +186,7 @@ GstElement
*GstreamerFrameCapture::get_encoder_plugin(const GstreamerEncoderSett
// Utility to add an element to a GstBin
// This checks return value and update reference correctly
-void gst_bin_add(GstBin *bin, const GstObjectUPtr<GstElement> &elem)
+void gst_bin_add(GstBin *bin, const GstElementUPtr &elem)
{
if (::gst_bin_add(bin, elem.get())) {
// ::gst_bin_add take ownership using floating references but
@@ -226,24 +202,24 @@ void GstreamerFrameCapture::pipeline_init(const
GstreamerEncoderSettings &settin
{
gboolean link;
- GstObjectUPtr<GstElement> pipeline(gst_pipeline_new("pipeline"));
+ GstElementUPtr pipeline(gst_pipeline_new("pipeline"));
if (!pipeline) {
throw std::runtime_error("Gstreamer's pipeline element cannot be
created");
}
- GstObjectUPtr<GstElement> capture(get_capture_plugin(settings));
+ GstElementUPtr capture(get_capture_plugin(settings));
if (!capture) {
throw std::runtime_error("Gstreamer's capture element cannot be
created");
}
- GstObjectUPtr<GstElement>
convert(gst_element_factory_make("autovideoconvert", "convert"));
+ GstElementUPtr convert(gst_element_factory_make("autovideoconvert",
"convert"));
if (!convert) {
throw std::runtime_error("Gstreamer's 'autovideoconvert' element
cannot be created");
}
GstCapsUPtr sink_caps;
- GstObjectUPtr<GstElement> encoder(get_encoder_plugin(settings,
sink_caps));
+ GstElementUPtr encoder(get_encoder_plugin(settings, sink_caps));
if (!encoder) {
throw std::runtime_error("Gstreamer's encoder element cannot be
created");
}
- GstObjectUPtr<GstElement> sink(gst_element_factory_make("appsink",
"sink"));
+ GstElementUPtr sink(gst_element_factory_make("appsink", "sink"));
if (!sink) {
throw std::runtime_error("Gstreamer's appsink element cannot be
created");
}
Let's see what other people think about.
Looks good to me. Much shorter.
I basically suggested the same thing (but with a template if possible).
Uri.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel