[libcamera-devel,v2,0/3] Colour spaces
mbox series

Message ID 20210927123327.14554-1-david.plowman@raspberrypi.com
Headers show
Series
  • Colour spaces
Related show

Message

David Plowman Sept. 27, 2021, 12:33 p.m. UTC 
Hi again

Thanks for all the feedback on the previous version. There's not
really any change in functionality here though I have in the main
taken people's suggestions on board. Just a few notes on some of that:

* I've de-inlined some code. I've left operator!= inline, though, as
  there seemed to be some precedent for that.

* I haven't replaced that vector by a map because it would require us
  to have an ordering for ColorSpaces (operator< etc.). I seem to
  recall inventing arbitrary orderings in the past, I guess it depends
  which approach we dislike least... thoughts?

* The ColorSpace now gets updated after a tryFormat. Thanks for
  pointing that out.

One final question in relation to this: I was hoping not to list every
last colour space variant that exists in V4L2, mostly because many of
them seem quite esoteric to me. Or would we rather simply mirror
everything in V4L2 straight away?

Thanks!

David

David Plowman (3):
  libcamera: Add ColorSpace class
  libcamera: Support passing ColorSpaces to V4L2 drivers
  libcamera: pipeline: raspberrypi: Support colour spaces

 include/libcamera/color_space.h               |  83 +++++++
 include/libcamera/internal/v4l2_videodevice.h |   2 +
 include/libcamera/meson.build                 |   1 +
 include/libcamera/stream.h                    |   3 +
 src/libcamera/color_space.cpp                 | 207 ++++++++++++++++++
 src/libcamera/meson.build                     |   1 +
 .../pipeline/raspberrypi/raspberrypi.cpp      |  84 +++++++
 src/libcamera/v4l2_videodevice.cpp            | 119 ++++++++++
 8 files changed, 500 insertions(+)
 create mode 100644 include/libcamera/color_space.h
 create mode 100644 src/libcamera/color_space.cpp

Comments

Laurent Pinchart Oct. 5, 2021, 12:48 p.m. UTC | #1 
Hi David,

On Mon, Sep 27, 2021 at 01:33:24PM +0100, David Plowman wrote:
> Hi again
> 
> Thanks for all the feedback on the previous version. There's not
> really any change in functionality here though I have in the main
> taken people's suggestions on board. Just a few notes on some of that:
> 
> * I've de-inlined some code. I've left operator!= inline, though, as
>   there seemed to be some precedent for that.
> 
> * I haven't replaced that vector by a map because it would require us
>   to have an ordering for ColorSpaces (operator< etc.). I seem to
>   recall inventing arbitrary orderings in the past, I guess it depends
>   which approach we dislike least... thoughts?

How about using std::unordered_map<> ?

> * The ColorSpace now gets updated after a tryFormat. Thanks for
>   pointing that out.
> 
> One final question in relation to this: I was hoping not to list every
> last colour space variant that exists in V4L2, mostly because many of
> them seem quite esoteric to me. Or would we rather simply mirror
> everything in V4L2 straight away?

That's fine with me as long as we make this extensible.

> David Plowman (3):
>   libcamera: Add ColorSpace class
>   libcamera: Support passing ColorSpaces to V4L2 drivers
>   libcamera: pipeline: raspberrypi: Support colour spaces
> 
>  include/libcamera/color_space.h               |  83 +++++++
>  include/libcamera/internal/v4l2_videodevice.h |   2 +
>  include/libcamera/meson.build                 |   1 +
>  include/libcamera/stream.h                    |   3 +
>  src/libcamera/color_space.cpp                 | 207 ++++++++++++++++++
>  src/libcamera/meson.build                     |   1 +
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  84 +++++++
>  src/libcamera/v4l2_videodevice.cpp            | 119 ++++++++++
>  8 files changed, 500 insertions(+)
>  create mode 100644 include/libcamera/color_space.h
>  create mode 100644 src/libcamera/color_space.cpp