[libcamera-devel] [PATCH v2 02/14] libcamera: v4l2_subdevice: Define format enumeration type
Jacopo Mondi
jacopo at jmondi.org
Thu Mar 14 11:10:20 CET 2019
Hi Kieran,
On Tue, Mar 12, 2019 at 12:22:46PM +0000, Kieran Bingham wrote:
> Hi Jacopo,
>
> On 12/03/2019 12:12, Jacopo Mondi wrote:
> > Provide a type definition for the map used to enumerate the subdevice
> > image formats and associated sizes.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/include/v4l2_subdevice.h | 5 +++--
> > src/libcamera/v4l2_subdevice.cpp | 14 +++++++++++---
> > 2 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h
> > index 700e66bcddac..15add95a825c 100644
> > --- a/src/libcamera/include/v4l2_subdevice.h
> > +++ b/src/libcamera/include/v4l2_subdevice.h
> > @@ -17,6 +17,8 @@
> >
> > namespace libcamera {
> >
> > +typedef std::map<unsigned int, std::vector<SizeRange>> SubdevFormatEnum;
>
> Should this have a V4L2 prefix? or is that overkill?
>
I accept suggestions. To me it's an overkill, but I understand formats
and V4L2-related APIs are "V4L2" prefixed.
What would you do here?
>
> > +
> > struct V4L2SubdeviceFormat {
> > uint32_t mbus_code;
> > uint32_t width;
> > @@ -42,8 +44,7 @@ public:
> > int setCrop(unsigned int pad, Rectangle *rect);
> > int setCompose(unsigned int pad, Rectangle *rect);
> >
> > - const std::map<unsigned int, std::vector<SizeRange>>
> > - formats(unsigned int pad);
> > + const SubdevFormatEnum formats(unsigned int pad);
> >
> > int getFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format);
> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> > index 7c56ba3ba510..153330bccc97 100644
> > --- a/src/libcamera/v4l2_subdevice.cpp
> > +++ b/src/libcamera/v4l2_subdevice.cpp
> > @@ -28,6 +28,15 @@ namespace libcamera {
> >
> > LOG_DEFINE_CATEGORY(V4L2Subdev)
> >
> > +/**
> > + * \typedef SubdevFormatEnum
> > + * \brief Type definition for the map of image formats and associated sizes
> > + *
> > + * Type definition for the map of media bus codes and associated image
> > + * resolutions used by V4L2Subdevice to report the enumeration of supported
> > + * image formats.
>
> Should/Could we document how to use this new 'type' in here somehow as
> the underlying type is now a bit obfuscated?
>
Not sure heither how to explain this better than looking at the type,
and seeing it's a map which associates media bus code to a vector of
image resolutions. Ah, see! I just explained what the new type
obscures. Should I add this?
Thanks
j
>
> > + */
> > +
> > /**
> > * \struct V4L2SubdeviceFormat
> > * \brief The V4L2 sub-device image format and sizes
> > @@ -210,10 +219,9 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)
> > * \return A map of image formats associated with a list of image sizes, or
> > * an empty map on error or if the pad does not exist
> > */
> > -const std::map<unsigned int, std::vector<SizeRange>>
> > -V4L2Subdevice::formats(unsigned int pad)
> > +const SubdevFormatEnum V4L2Subdevice::formats(unsigned int pad)
> > {
> > - std::map<unsigned int, std::vector<SizeRange>> formatMap = {};
> > + SubdevFormatEnum formatMap = {};
> > struct v4l2_subdev_mbus_code_enum mbusEnum = {};
> > int ret;
> >
> >
>
> --
> Regards
> --
> Kieran
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20190314/9da41bb9/attachment.sig>
More information about the libcamera-devel
mailing list