| Message ID | 20200129033210.278800-9-nicolas@ndufresne.ca |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Nicolas, Thank you for the patch. On Tue, Jan 28, 2020 at 10:31:55PM -0500, Nicolas Dufresne wrote: > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > This will allow selecting libcamerasrc traces with the following > environment: > > GST_DEBUG=libcamerasrc:7 > > Or all libcamera GStreamer element traces using > > GST_DEBUG="libcamera*:7" > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > --- > src/gstreamer/gstlibcamerasrc.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp > index 74e1d7e..2177a8d 100644 > --- a/src/gstreamer/gstlibcamerasrc.cpp > +++ b/src/gstreamer/gstlibcamerasrc.cpp > @@ -10,6 +10,9 @@ > #include "gstlibcamerapad.h" > #include "gstlibcamera-utils.h" > > +GST_DEBUG_CATEGORY_STATIC(source_debug); > +#define GST_CAT_DEFAULT source_debug > + > struct _GstLibcameraSrc { > GstElement parent; > GstPad *srcpad; > @@ -21,7 +24,9 @@ enum { > PROP_CAMERA_NAME > }; > > -G_DEFINE_TYPE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT); > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT, > + GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0, > + "LibCamera Source")); s/LibCamera/libcamera/ (see branding comment in a previous patch). Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > #define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw;image/jpeg") >
On mar, 2020-02-11 at 21:29 +0200, Laurent Pinchart wrote: > Hi Nicolas, > > Thank you for the patch. > > On Tue, Jan 28, 2020 at 10:31:55PM -0500, Nicolas Dufresne wrote: > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > This will allow selecting libcamerasrc traces with the following > > environment: > > > > GST_DEBUG=libcamerasrc:7 > > > > Or all libcamera GStreamer element traces using > > > > GST_DEBUG="libcamera*:7" > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > --- > > src/gstreamer/gstlibcamerasrc.cpp | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp > > b/src/gstreamer/gstlibcamerasrc.cpp > > index 74e1d7e..2177a8d 100644 > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > @@ -10,6 +10,9 @@ > > #include "gstlibcamerapad.h" > > #include "gstlibcamera-utils.h" > > > > +GST_DEBUG_CATEGORY_STATIC(source_debug); > > +#define GST_CAT_DEFAULT source_debug > > + > > struct _GstLibcameraSrc { > > GstElement parent; > > GstPad *srcpad; > > @@ -21,7 +24,9 @@ enum { > > PROP_CAMERA_NAME > > }; > > > > -G_DEFINE_TYPE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT); > > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, > > GST_TYPE_ELEMENT, > > + GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0, > > + "LibCamera Source")); > > s/LibCamera/libcamera/ (see branding comment in a previous patch). As per GStreamer style, I would need to use snake gst_lib_camera_ then, are you ok with that change ? I'm still wondering if I should try and find a nickname, but haven't really found a good one, or any guidelines. > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > #define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw;image/jpeg") > >
Hi Nicolas, On Tue, Feb 11, 2020 at 04:57:28PM -0500, Nicolas Dufresne wrote: > On mar, 2020-02-11 at 21:29 +0200, Laurent Pinchart wrote: > > On Tue, Jan 28, 2020 at 10:31:55PM -0500, Nicolas Dufresne wrote: > > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > This will allow selecting libcamerasrc traces with the following > > > environment: > > > > > > GST_DEBUG=libcamerasrc:7 > > > > > > Or all libcamera GStreamer element traces using > > > > > > GST_DEBUG="libcamera*:7" > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > --- > > > src/gstreamer/gstlibcamerasrc.cpp | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp > > > b/src/gstreamer/gstlibcamerasrc.cpp > > > index 74e1d7e..2177a8d 100644 > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > @@ -10,6 +10,9 @@ > > > #include "gstlibcamerapad.h" > > > #include "gstlibcamera-utils.h" > > > > > > +GST_DEBUG_CATEGORY_STATIC(source_debug); > > > +#define GST_CAT_DEFAULT source_debug > > > + > > > struct _GstLibcameraSrc { > > > GstElement parent; > > > GstPad *srcpad; > > > @@ -21,7 +24,9 @@ enum { > > > PROP_CAMERA_NAME > > > }; > > > > > > -G_DEFINE_TYPE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT); > > > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, > > > GST_TYPE_ELEMENT, > > > + GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0, > > > + "LibCamera Source")); > > > > s/LibCamera/libcamera/ (see branding comment in a previous patch). > > As per GStreamer style, I would need to use snake gst_lib_camera_ then, are you > ok with that change ? Even for the last string (that would become "libcamera Source") ? Libcamera with a capital L is fine in class names such as GstLibcameraSrc, it's in user-visible text that we try to keep it lower-case (and in documentation, comments, commit messages, ...). > I'm still wondering if I should try and find a nickname, > but haven't really found a good one, or any guidelines. I haven't really found one either, but haven't spent too much time on it. The libcamera namespace prefix, in particular, is a bit long. Abbreviating it to libc isn't a very good idea :-) > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > #define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw;image/jpeg")
On mer, 2020-02-12 at 01:09 +0200, Laurent Pinchart wrote: > Hi Nicolas, > > On Tue, Feb 11, 2020 at 04:57:28PM -0500, Nicolas Dufresne wrote: > > On mar, 2020-02-11 at 21:29 +0200, Laurent Pinchart wrote: > > > On Tue, Jan 28, 2020 at 10:31:55PM -0500, Nicolas Dufresne wrote: > > > > From: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > > > > > This will allow selecting libcamerasrc traces with the following > > > > environment: > > > > > > > > GST_DEBUG=libcamerasrc:7 > > > > > > > > Or all libcamera GStreamer element traces using > > > > > > > > GST_DEBUG="libcamera*:7" > > > > > > > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> > > > > --- > > > > src/gstreamer/gstlibcamerasrc.cpp | 7 ++++++- > > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp > > > > b/src/gstreamer/gstlibcamerasrc.cpp > > > > index 74e1d7e..2177a8d 100644 > > > > --- a/src/gstreamer/gstlibcamerasrc.cpp > > > > +++ b/src/gstreamer/gstlibcamerasrc.cpp > > > > @@ -10,6 +10,9 @@ > > > > #include "gstlibcamerapad.h" > > > > #include "gstlibcamera-utils.h" > > > > > > > > +GST_DEBUG_CATEGORY_STATIC(source_debug); > > > > +#define GST_CAT_DEFAULT source_debug > > > > + > > > > struct _GstLibcameraSrc { > > > > GstElement parent; > > > > GstPad *srcpad; > > > > @@ -21,7 +24,9 @@ enum { > > > > PROP_CAMERA_NAME > > > > }; > > > > > > > > -G_DEFINE_TYPE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT); > > > > +G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, > > > > GST_TYPE_ELEMENT, > > > > + GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0, > > > > + "LibCamera Source")); > > > > > > s/LibCamera/libcamera/ (see branding comment in a previous patch). > > > > As per GStreamer style, I would need to use snake gst_lib_camera_ then, are you > > ok with that change ? > > Even for the last string (that would become "libcamera Source") ? > Libcamera with a capital L is fine in class names such as > GstLibcameraSrc, it's in user-visible text that we try to keep it > lower-case (and in documentation, comments, commit messages, ...). /o\ Got it. I miss-read, and didn't realized you mean the other way around. Ok, I'll fix the debug category descriprion. I'll check the element meta just to make sure. > > > I'm still wondering if I should try and find a nickname, > > but haven't really found a good one, or any guidelines. > > I haven't really found one either, but haven't spent too much time on > it. The libcamera namespace prefix, in particular, is a bit long. > Abbreviating it to libc isn't a very good idea :-) Lol, would be rather confusing. For the element name, I also considered camerasrc, but that felt a bit pretentious even if the name is free. It might would be confusing with camerasrcwrapper and camerasrcbin though (the Nokia GStreamer API for cameras, with the GstPhotography interface). > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > > #define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw;image/jpeg")
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 74e1d7e..2177a8d 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -10,6 +10,9 @@ #include "gstlibcamerapad.h" #include "gstlibcamera-utils.h" +GST_DEBUG_CATEGORY_STATIC(source_debug); +#define GST_CAT_DEFAULT source_debug + struct _GstLibcameraSrc { GstElement parent; GstPad *srcpad; @@ -21,7 +24,9 @@ enum { PROP_CAMERA_NAME }; -G_DEFINE_TYPE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT); +G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT, + GST_DEBUG_CATEGORY_INIT(source_debug, "libcamerasrc", 0, + "LibCamera Source")); #define TEMPLATE_CAPS GST_STATIC_CAPS("video/x-raw;image/jpeg")