| Message ID | 20201223121055.14178-1-sebastian.fricke.linux@gmail.com |
|---|---|
| Headers | show |
| Series |
|
| Related | show |
Hi Sebastian, On Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote: > This patch series adds unit-tests and the `fromV4L2PixelFormat` method > to the BayerFormat class. > > I also wanted to discuss one alternative that I played around with. > We could maybe drop one of the two mapping tables and use the following > logic to get the map key from a mapped value. > > ``` > #include <algorithm> > ... > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const > { > auto it = std::find_if( > bayerToV4l2.begin(), > bayerToV4l2.end(), > [v4l2Format](const auto& i) { return i->second == v4l2Format; } > ); > if (it != bayerToV4l2.end()) > return it->first; > > return BayerFormat(); > } > ``` This would certainly work. The question is whether we can to optimize the memory consumption (dropping the second lookup table), or the execution time (a lookup operation for a map has a O(log(n)) complexity, compared to O(n) for the code above). > Sebastian Fricke (2): > libcamera: Add the fromV4L2PixelFormat method > test: Add unit tests for the BayerFormat class > > include/libcamera/internal/bayer_format.h | 1 + > src/libcamera/bayer_format.cpp | 14 ++ > test/bayer_format.cpp | 154 ++++++++++++++++++++++ > test/meson.build | 1 + > 4 files changed, 170 insertions(+) > create mode 100644 test/bayer_format.cpp >
On 26.12.2020 00:06, Laurent Pinchart wrote: >Hi Sebastian, Hey Laurent, > >On Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote: >> This patch series adds unit-tests and the `fromV4L2PixelFormat` method >> to the BayerFormat class. >> >> I also wanted to discuss one alternative that I played around with. >> We could maybe drop one of the two mapping tables and use the following >> logic to get the map key from a mapped value. >> >> ``` >> #include <algorithm> >> ... >> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const >> { >> auto it = std::find_if( >> bayerToV4l2.begin(), >> bayerToV4l2.end(), >> [v4l2Format](const auto& i) { return i->second == v4l2Format; } >> ); >> if (it != bayerToV4l2.end()) >> return it->first; >> >> return BayerFormat(); >> } >> ``` > >This would certainly work. The question is whether we can to optimize >the memory consumption (dropping the second lookup table), or the >execution time (a lookup operation for a map has a O(log(n)) complexity, >compared to O(n) for the code above). Which probably boils down to the question: 'How often do we expect to use this function?'. Can you recall from the current code bases if there are cases, where the constructor is used frequently in a row? I would suspect that the constructor/static function should be used quite seldomly. > >> Sebastian Fricke (2): >> libcamera: Add the fromV4L2PixelFormat method >> test: Add unit tests for the BayerFormat class >> >> include/libcamera/internal/bayer_format.h | 1 + >> src/libcamera/bayer_format.cpp | 14 ++ >> test/bayer_format.cpp | 154 ++++++++++++++++++++++ >> test/meson.build | 1 + >> 4 files changed, 170 insertions(+) >> create mode 100644 test/bayer_format.cpp >> > >-- >Regards, > >Laurent Pinchart
Hi Sebastian, On Mon, Dec 28, 2020 at 08:05:28AM +0100, Sebastian Fricke wrote: > On 26.12.2020 00:06, Laurent Pinchart wrote: > >On Wed, Dec 23, 2020 at 01:10:53PM +0100, Sebastian Fricke wrote: > >> This patch series adds unit-tests and the `fromV4L2PixelFormat` method > >> to the BayerFormat class. > >> > >> I also wanted to discuss one alternative that I played around with. > >> We could maybe drop one of the two mapping tables and use the following > >> logic to get the map key from a mapped value. > >> > >> ``` > >> #include <algorithm> > >> ... > >> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const > >> { > >> auto it = std::find_if( > >> bayerToV4l2.begin(), > >> bayerToV4l2.end(), > >> [v4l2Format](const auto& i) { return i->second == v4l2Format; } > >> ); > >> if (it != bayerToV4l2.end()) > >> return it->first; > >> > >> return BayerFormat(); > >> } > >> ``` > > > > This would certainly work. The question is whether we can to optimize > > the memory consumption (dropping the second lookup table), or the > > execution time (a lookup operation for a map has a O(log(n)) complexity, > > compared to O(n) for the code above). > > Which probably boils down to the question: 'How often do we expect to > use this function?'. > Can you recall from the current code bases if there are cases, where the > constructor is used frequently in a row? I would suspect that the > constructor/static function should be used quite seldomly. I don't expect this to be used in any hot path. > >> Sebastian Fricke (2): > >> libcamera: Add the fromV4L2PixelFormat method > >> test: Add unit tests for the BayerFormat class > >> > >> include/libcamera/internal/bayer_format.h | 1 + > >> src/libcamera/bayer_format.cpp | 14 ++ > >> test/bayer_format.cpp | 154 ++++++++++++++++++++++ > >> test/meson.build | 1 + > >> 4 files changed, 170 insertions(+) > >> create mode 100644 test/bayer_format.cpp
This patch series adds unit-tests and the `fromV4L2PixelFormat` method to the BayerFormat class. I also wanted to discuss one alternative that I played around with. We could maybe drop one of the two mapping tables and use the following logic to get the map key from a mapped value. ``` #include <algorithm> ... BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format) const { auto it = std::find_if( bayerToV4l2.begin(), bayerToV4l2.end(), [v4l2Format](const auto& i) { return i->second == v4l2Format; } ); if (it != bayerToV4l2.end()) return it->first; return BayerFormat(); } ``` Sebastian Fricke (2): libcamera: Add the fromV4L2PixelFormat method test: Add unit tests for the BayerFormat class include/libcamera/internal/bayer_format.h | 1 + src/libcamera/bayer_format.cpp | 14 ++ test/bayer_format.cpp | 154 ++++++++++++++++++++++ test/meson.build | 1 + 4 files changed, 170 insertions(+) create mode 100644 test/bayer_format.cpp