| Message ID | 20200130213809.16564-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Jan 30, 2020 at 09:38:09PM +0000, Kieran Bingham wrote: > Provide a helper to print the FourCC string representation, and utilise > it to improve the readability of the StreamConfiguration. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > This changes the output of the line: > [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d > > to read: > [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d MJPG > > include/libcamera/pixelformats.h | 3 +++ > src/libcamera/pixelformats.cpp | 20 ++++++++++++++++++++ > src/libcamera/stream.cpp | 3 ++- > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h > index 6e25b8d8b76e..32cff083cdf9 100644 > --- a/include/libcamera/pixelformats.h > +++ b/include/libcamera/pixelformats.h > @@ -8,11 +8,14 @@ > #define __LIBCAMERA_PIXEL_FORMATS_H__ > > #include <stdint.h> > +#include <string> > > namespace libcamera { > > using PixelFormat = uint32_t; > > +std::string FourCC(const PixelFormat &p); > + I think we should turn PixelFormat into a struct or class, and add a toString() method. Moving away from the uint32_t alias will also disallow implicit conversions, which would have helped catch bugs in the past, and should help avoiding them in the future. > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */ > diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp > index c03335400b70..d6b2c6a3dba5 100644 > --- a/src/libcamera/pixelformats.cpp > +++ b/src/libcamera/pixelformats.cpp > @@ -7,6 +7,8 @@ > > #include <libcamera/pixelformats.h> > > +#include <sstream> > + > /** > * \file pixelformats.h > * \brief libcamera pixel formats > @@ -25,4 +27,22 @@ namespace libcamera { > * \todo Add support for format modifiers > */ > > +/** > + * \brief Return a PixelFormat as a FourCC string representation > + */ > +std::string FourCC(const PixelFormat &p) > +{ > + std::stringstream ss; > + > + ss << static_cast<char>(p & 0x7f) > + << static_cast<char>((p >> 8) & 0x7f) > + << static_cast<char>((p >> 16) & 0x7f) > + << static_cast<char>((p >> 24) & 0x7f); > + > + if (p & (1 << 31)) > + ss << "-BE"; > + > + return ss.str(); > +} > + > } /* namespace libcamera */ > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 13789e9eb344..4efe4385326f 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -348,7 +348,8 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) > std::string StreamConfiguration::toString() const > { > std::stringstream ss; > - ss << size.toString() << "-" << utils::hex(pixelFormat); > + ss << size.toString() << "-" << utils::hex(pixelFormat) > + << " " << FourCC(pixelFormat); > return ss.str(); > } >
Le jeudi 30 janvier 2020 à 21:38 +0000, Kieran Bingham a écrit : > Provide a helper to print the FourCC string representation, and utilise > it to improve the readability of the StreamConfiguration. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > This changes the output of the line: > [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d > > to read: > [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d MJPG Perhaps we could drop the hex form ? Is it really useful ? > > include/libcamera/pixelformats.h | 3 +++ > src/libcamera/pixelformats.cpp | 20 ++++++++++++++++++++ > src/libcamera/stream.cpp | 3 ++- > 3 files changed, 25 insertions(+), 1 deletion(-) > > diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h > index 6e25b8d8b76e..32cff083cdf9 100644 > --- a/include/libcamera/pixelformats.h > +++ b/include/libcamera/pixelformats.h > @@ -8,11 +8,14 @@ > #define __LIBCAMERA_PIXEL_FORMATS_H__ > > #include <stdint.h> > +#include <string> > > namespace libcamera { > > using PixelFormat = uint32_t; > > +std::string FourCC(const PixelFormat &p); > + > } /* namespace libcamera */ > > #endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */ > diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp > index c03335400b70..d6b2c6a3dba5 100644 > --- a/src/libcamera/pixelformats.cpp > +++ b/src/libcamera/pixelformats.cpp > @@ -7,6 +7,8 @@ > > #include <libcamera/pixelformats.h> > > +#include <sstream> > + > /** > * \file pixelformats.h > * \brief libcamera pixel formats > @@ -25,4 +27,22 @@ namespace libcamera { > * \todo Add support for format modifiers > */ > > +/** > + * \brief Return a PixelFormat as a FourCC string representation > + */ > +std::string FourCC(const PixelFormat &p) > +{ > + std::stringstream ss; > + > + ss << static_cast<char>(p & 0x7f) > + << static_cast<char>((p >> 8) & 0x7f) > + << static_cast<char>((p >> 16) & 0x7f) > + << static_cast<char>((p >> 24) & 0x7f); > + > + if (p & (1 << 31)) > + ss << "-BE"; > + > + return ss.str(); > +} > + > } /* namespace libcamera */ > diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp > index 13789e9eb344..4efe4385326f 100644 > --- a/src/libcamera/stream.cpp > +++ b/src/libcamera/stream.cpp > @@ -348,7 +348,8 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) > std::string StreamConfiguration::toString() const > { > std::stringstream ss; > - ss << size.toString() << "-" << utils::hex(pixelFormat); > + ss << size.toString() << "-" << utils::hex(pixelFormat) > + << " " << FourCC(pixelFormat); > return ss.str(); > } >
Hi Nicolas, On 31/01/2020 15:20, Nicolas Dufresne wrote: > Le jeudi 30 janvier 2020 à 21:38 +0000, Kieran Bingham a écrit : >> Provide a helper to print the FourCC string representation, and utilise >> it to improve the readability of the StreamConfiguration. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> >> --- >> This changes the output of the line: >> [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d >> >> to read: >> [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d MJPG > > Perhaps we could drop the hex form ? Is it really useful ? Indeed it probably isn't! I'll remove this as part of the respin to a class/struct as suggested by Laurent. -- Kieran >> >> include/libcamera/pixelformats.h | 3 +++ >> src/libcamera/pixelformats.cpp | 20 ++++++++++++++++++++ >> src/libcamera/stream.cpp | 3 ++- >> 3 files changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h >> index 6e25b8d8b76e..32cff083cdf9 100644 >> --- a/include/libcamera/pixelformats.h >> +++ b/include/libcamera/pixelformats.h >> @@ -8,11 +8,14 @@ >> #define __LIBCAMERA_PIXEL_FORMATS_H__ >> >> #include <stdint.h> >> +#include <string> >> >> namespace libcamera { >> >> using PixelFormat = uint32_t; >> >> +std::string FourCC(const PixelFormat &p); >> + >> } /* namespace libcamera */ >> >> #endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */ >> diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp >> index c03335400b70..d6b2c6a3dba5 100644 >> --- a/src/libcamera/pixelformats.cpp >> +++ b/src/libcamera/pixelformats.cpp >> @@ -7,6 +7,8 @@ >> >> #include <libcamera/pixelformats.h> >> >> +#include <sstream> >> + >> /** >> * \file pixelformats.h >> * \brief libcamera pixel formats >> @@ -25,4 +27,22 @@ namespace libcamera { >> * \todo Add support for format modifiers >> */ >> >> +/** >> + * \brief Return a PixelFormat as a FourCC string representation >> + */ >> +std::string FourCC(const PixelFormat &p) >> +{ >> + std::stringstream ss; >> + >> + ss << static_cast<char>(p & 0x7f) >> + << static_cast<char>((p >> 8) & 0x7f) >> + << static_cast<char>((p >> 16) & 0x7f) >> + << static_cast<char>((p >> 24) & 0x7f); >> + >> + if (p & (1 << 31)) >> + ss << "-BE"; >> + >> + return ss.str(); >> +} >> + >> } /* namespace libcamera */ >> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp >> index 13789e9eb344..4efe4385326f 100644 >> --- a/src/libcamera/stream.cpp >> +++ b/src/libcamera/stream.cpp >> @@ -348,7 +348,8 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) >> std::string StreamConfiguration::toString() const >> { >> std::stringstream ss; >> - ss << size.toString() << "-" << utils::hex(pixelFormat); >> + ss << size.toString() << "-" << utils::hex(pixelFormat) >> + << " " << FourCC(pixelFormat); >> return ss.str(); >> } >> >
diff --git a/include/libcamera/pixelformats.h b/include/libcamera/pixelformats.h index 6e25b8d8b76e..32cff083cdf9 100644 --- a/include/libcamera/pixelformats.h +++ b/include/libcamera/pixelformats.h @@ -8,11 +8,14 @@ #define __LIBCAMERA_PIXEL_FORMATS_H__ #include <stdint.h> +#include <string> namespace libcamera { using PixelFormat = uint32_t; +std::string FourCC(const PixelFormat &p); + } /* namespace libcamera */ #endif /* __LIBCAMERA_PIXEL_FORMATS_H__ */ diff --git a/src/libcamera/pixelformats.cpp b/src/libcamera/pixelformats.cpp index c03335400b70..d6b2c6a3dba5 100644 --- a/src/libcamera/pixelformats.cpp +++ b/src/libcamera/pixelformats.cpp @@ -7,6 +7,8 @@ #include <libcamera/pixelformats.h> +#include <sstream> + /** * \file pixelformats.h * \brief libcamera pixel formats @@ -25,4 +27,22 @@ namespace libcamera { * \todo Add support for format modifiers */ +/** + * \brief Return a PixelFormat as a FourCC string representation + */ +std::string FourCC(const PixelFormat &p) +{ + std::stringstream ss; + + ss << static_cast<char>(p & 0x7f) + << static_cast<char>((p >> 8) & 0x7f) + << static_cast<char>((p >> 16) & 0x7f) + << static_cast<char>((p >> 24) & 0x7f); + + if (p & (1 << 31)) + ss << "-BE"; + + return ss.str(); +} + } /* namespace libcamera */ diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp index 13789e9eb344..4efe4385326f 100644 --- a/src/libcamera/stream.cpp +++ b/src/libcamera/stream.cpp @@ -348,7 +348,8 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats) std::string StreamConfiguration::toString() const { std::stringstream ss; - ss << size.toString() << "-" << utils::hex(pixelFormat); + ss << size.toString() << "-" << utils::hex(pixelFormat) + << " " << FourCC(pixelFormat); return ss.str(); }
Provide a helper to print the FourCC string representation, and utilise it to improve the readability of the StreamConfiguration. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- This changes the output of the line: [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d to read: [159:34:26.262068766] [16497] INFO Camera camera.cpp:784 configuring streams: (0) 1920x1080-0x47504a4d MJPG include/libcamera/pixelformats.h | 3 +++ src/libcamera/pixelformats.cpp | 20 ++++++++++++++++++++ src/libcamera/stream.cpp | 3 ++- 3 files changed, 25 insertions(+), 1 deletion(-)