* [PATCH 1/8] clang-tidy: use auto
@ 2021-04-21 7:20 Rosen Penev
2021-04-21 7:20 ` [PATCH 2/8] clang-tidy: use nullptr Rosen Penev
` (6 more replies)
0 siblings, 7 replies; 17+ messages in thread
From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw)
To: linux-media
Found with modernize-use-auto
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
utils/cec-ctl/cec-ctl.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/utils/cec-ctl/cec-ctl.cpp b/utils/cec-ctl/cec-ctl.cpp
index 107dcfab2..c17265a5c 100644
--- a/utils/cec-ctl/cec-ctl.cpp
+++ b/utils/cec-ctl/cec-ctl.cpp
@@ -1833,7 +1833,7 @@ static __u16 parse_phys_addr_from_edid(const char *edid_path)
static void *thread_edid_poll(void *arg)
{
- struct node *node = static_cast<struct node *>(arg);
+ auto node = static_cast<struct node *>(arg);
__u16 phys_addr;
bool has_edid;
char dummy;
--
2.30.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/8] clang-tidy: use nullptr 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 7:20 ` [PATCH 3/8] remove unused ARRAY_SIZE Rosen Penev ` (5 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media Found with modernize-use-nullptr Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/libcecutil/cec-info.cpp | 2 +- utils/v4l2-compliance/v4l2-compliance.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/utils/libcecutil/cec-info.cpp b/utils/libcecutil/cec-info.cpp index 22a687761..0172e8537 100644 --- a/utils/libcecutil/cec-info.cpp +++ b/utils/libcecutil/cec-info.cpp @@ -168,7 +168,7 @@ const char *cec_vendor2s(unsigned vendor) case 0x8065e9: return "Benq"; default: - return NULL; + return nullptr; } } diff --git a/utils/v4l2-compliance/v4l2-compliance.cpp b/utils/v4l2-compliance/v4l2-compliance.cpp index 9f71332ce..90a503612 100644 --- a/utils/v4l2-compliance/v4l2-compliance.cpp +++ b/utils/v4l2-compliance/v4l2-compliance.cpp @@ -1460,7 +1460,7 @@ void testNode(struct node &node, struct node &node_m2m_cap, struct node &expbuf_ node.controls.find(VIVID_CID_DISCONNECT) != node.controls.end()) { if (node.node2) node.node2->close(); - node.node2 = NULL; + node.node2 = nullptr; printf("\ttest Disconnect: %s\n\n", ok(testVividDisconnect(&node))); } -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/8] remove unused ARRAY_SIZE 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev 2021-04-21 7:20 ` [PATCH 2/8] clang-tidy: use nullptr Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 7:20 ` [PATCH 4/8] cec-tuner: std::array conversions Rosen Penev ` (4 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media This is a C construct that is not really needed in C++. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/cec-compliance/cec-compliance.h | 3 --- utils/cec-ctl/cec-pin.cpp | 2 -- utils/cec-follower/cec-follower.h | 3 --- utils/cec-follower/cec-processing.cpp | 3 --- utils/rds-ctl/rds-ctl.cpp | 2 -- 5 files changed, 13 deletions(-) diff --git a/utils/cec-compliance/cec-compliance.h b/utils/cec-compliance/cec-compliance.h index 26c70c216..c558f043b 100644 --- a/utils/cec-compliance/cec-compliance.h +++ b/utils/cec-compliance/cec-compliance.h @@ -191,9 +191,6 @@ using vec_remote_subtests = std::vector<remote_subtest>; #define CEC_LOG_ADDR_MASK_ALL 0xffff -#define ARRAY_SIZE(a) \ - (sizeof(a) / sizeof(*a)) - #define COLOR_GREEN(s) "\033[32m" s "\033[0m" #define COLOR_RED(s) "\033[1;31m" s "\033[0m" #define COLOR_BOLD(s) "\033[1m" s "\033[0m" diff --git a/utils/cec-ctl/cec-pin.cpp b/utils/cec-ctl/cec-pin.cpp index f9ecac7fb..30e5accf5 100644 --- a/utils/cec-ctl/cec-pin.cpp +++ b/utils/cec-ctl/cec-pin.cpp @@ -17,8 +17,6 @@ #include "cec-ctl.h" #include "cec-log.h" -#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0])) - static std::string find_opcode_name(__u8 opcode) { const char *name = cec_opcode2s(opcode); diff --git a/utils/cec-follower/cec-follower.h b/utils/cec-follower/cec-follower.h index 5b67a260f..7806a4b64 100644 --- a/utils/cec-follower/cec-follower.h +++ b/utils/cec-follower/cec-follower.h @@ -20,9 +20,6 @@ #include <cec-info.h> #include <cec-log.h> -#define ARRAY_SIZE(a) \ - (sizeof(a) / sizeof(*a)) - extern bool show_info; extern bool show_msgs; extern bool show_state; diff --git a/utils/cec-follower/cec-processing.cpp b/utils/cec-follower/cec-processing.cpp index 83ffcc30b..024407471 100644 --- a/utils/cec-follower/cec-processing.cpp +++ b/utils/cec-follower/cec-processing.cpp @@ -29,9 +29,6 @@ /* Time between each polling message sent to a device */ #define POLL_PERIOD 15000 -#define ARRAY_SIZE(a) \ - (sizeof(a) / sizeof(*a)) - struct cec_enum_values { const char *type_name; __u8 value; diff --git a/utils/rds-ctl/rds-ctl.cpp b/utils/rds-ctl/rds-ctl.cpp index e27182821..8161aa453 100644 --- a/utils/rds-ctl/rds-ctl.cpp +++ b/utils/rds-ctl/rds-ctl.cpp @@ -27,8 +27,6 @@ #include <linux/videodev2.h> #include <libv4l2rds.h> -#define ARRAY_SIZE(arr) ((int)(sizeof(arr) / sizeof((arr)[0]))) - using dev_vec = std::vector<std::string>; using dev_map = std::map<std::string, std::string>; -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/8] cec-tuner: std::array conversions 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev 2021-04-21 7:20 ` [PATCH 2/8] clang-tidy: use nullptr Rosen Penev 2021-04-21 7:20 ` [PATCH 3/8] remove unused ARRAY_SIZE Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 7:20 ` [PATCH 5/8] v4l2-info: remove a strange sizeof usage Rosen Penev ` (3 subsequent siblings) 6 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media Allows getting rid of sizeof macros. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/cec-follower/cec-tuner.cpp | 140 +++++++++++++++---------------- 1 file changed, 70 insertions(+), 70 deletions(-) diff --git a/utils/cec-follower/cec-tuner.cpp b/utils/cec-follower/cec-tuner.cpp index ac5c49950..b9c21684b 100644 --- a/utils/cec-follower/cec-tuner.cpp +++ b/utils/cec-follower/cec-tuner.cpp @@ -3,6 +3,7 @@ * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved. */ +#include <array> #include <ctime> #include <string> @@ -13,10 +14,8 @@ #define NUM_ANALOG_FREQS 3 #define NUM_DIGITAL_CHANS 3 -#define TOT_ANALOG_FREQS (sizeof(analog_freqs_khz) / sizeof(analog_freqs_khz[0][0][0])) -#define TOT_DIGITAL_CHANS ((sizeof(digital_arib_data) / sizeof(digital_arib_data[0][0])) + \ - (sizeof(digital_atsc_data) / sizeof(digital_atsc_data[0][0])) + \ - (sizeof(digital_dvb_data) / sizeof(digital_dvb_data[0][0]))) +#define TOT_ANALOG_FREQS analog_freqs_khz[0][0].size() +#define TOT_DIGITAL_CHANS digital_arib_data[0].size() + digital_atsc_data[0].size() + digital_dvb_data[0].size() struct service_info { unsigned tsid; @@ -45,22 +44,22 @@ struct service_info { * * https://sichbopvr.com/frequency-tables/Brazil/Rio%20de%20Janeiro/Rio%20De%20Janeiro */ -static const struct service_info digital_arib_data[2][NUM_DIGITAL_CHANS] = -{ +using arib = std::array<service_info, NUM_DIGITAL_CHANS>; +static constexpr std::array<arib, 2> digital_arib_data{ // satellite, arib-bs - { + arib{ // tsid, onid, sid, fmt, major, minor - { 1032, 1, 30203, 1, 0, 30203 }, - { 1046, 1, 30505, 1, 0, 30505 }, - { 1060, 1, 30609, 1, 0, 30609 } + service_info{ 1032, 1, 30203, 1, 0, 30203 }, + service_info{ 1046, 1, 30505, 1, 0, 30505 }, + service_info{ 1060, 1, 30609, 1, 0, 30609 }, }, // terrestrial, arib-t - { + arib{ // tsid, onid, sid, fmt, major, minor - { 1519, 1519, 48608, 1, 0, 48608 }, - { 1624, 1624, 51992, 1, 0, 51992 }, - { 1905, 1905, 60960, 1, 0, 60960 } - } + service_info{ 1519, 1519, 48608, 1, 0, 48608 }, + service_info{ 1624, 1624, 51992, 1, 0, 51992 }, + service_info{ 1905, 1905, 60960, 1, 0, 60960 }, + }, }; /* @@ -84,22 +83,22 @@ static const struct service_info digital_arib_data[2][NUM_DIGITAL_CHANS] = * ATSC does not use ONIDs and SID will be used as the program number. All ATSC * channel number formats are 2 part. */ -static const struct service_info digital_atsc_data[2][NUM_DIGITAL_CHANS] = -{ +using atsc = std::array<service_info, NUM_DIGITAL_CHANS>; +static constexpr std::array<atsc, 2> digital_atsc_data{ // satellite, atsc-sat - { + atsc{ // tsid, onid, sid, fmt, major, minor - { 2065, 0, 50316, 2, 3, 50316 }, - { 2090, 0, 50882, 2, 3, 50882 }, - { 2122, 0, 55295, 2, 3, 55295 } + service_info{ 2065, 0, 50316, 2, 3, 50316 }, + service_info{ 2090, 0, 50882, 2, 3, 50882 }, + service_info{ 2122, 0, 55295, 2, 3, 55295 }, }, // terrestrial, atsc-t - { + atsc{ // tsid, onid, sid, fmt, major, minor - { 1675, 0, 1, 2, 4, 1 }, - { 1675, 0, 2, 2, 4, 2 }, - { 1675, 0, 3, 2, 4, 3 } - } + service_info{ 1675, 0, 1, 2, 4, 1 }, + service_info{ 1675, 0, 2, 2, 4, 2 }, + service_info{ 1675, 0, 3, 2, 4, 3 }, + }, }; /* @@ -119,22 +118,22 @@ static const struct service_info digital_atsc_data[2][NUM_DIGITAL_CHANS] = * https://sichbopvr.com/frequency-tables/Sweden/Skane%20Lan/Malm%c3%b6 * */ -static const struct service_info digital_dvb_data[2][NUM_DIGITAL_CHANS] = -{ +using dvb = std::array<service_info, NUM_DIGITAL_CHANS>; +static constexpr std::array<dvb, 2> digital_dvb_data{ // satellite, dvb-s2 - { + dvb{ // tsid, onid, sid, fmt, major, minor - { 61, 70, 7193, 1, 0, 24 }, - { 65, 70, 7040, 1, 0, 72 }, - { 28, 70, 7012, 1, 0, 101 } + service_info{ 61, 70, 7193, 1, 0, 24 }, + service_info{ 65, 70, 7040, 1, 0, 72 }, + service_info{ 28, 70, 7012, 1, 0, 101 }, }, // terrestrial, dvb-t - { + dvb{ // tsid, onid, sid, fmt, major, minor - { 1002, 8400, 2025, 1, 0, 21 }, - { 1004, 8400, 84, 1, 0, 31 }, - { 1004, 8945, 1040, 1, 0, 1040 } - } + service_info{ 1002, 8400, 2025, 1, 0, 21 }, + service_info{ 1004, 8400, 84, 1, 0, 31 }, + service_info{ 1004, 8945, 1040, 1, 0, 1040 }, + }, }; /* @@ -150,71 +149,72 @@ static const struct service_info digital_dvb_data[2][NUM_DIGITAL_CHANS] = * * https://en.wikipedia.org/wiki/Television_channel_frequencies */ -static unsigned int analog_freqs_khz[3][9][NUM_ANALOG_FREQS] = -{ +using khz = std::array<unsigned int, NUM_ANALOG_FREQS>; +using freqs = std::array<khz, 9>; +static constexpr std::array<freqs, 3> analog_freqs_khz{ // cable - { + freqs{ // pal-bg - { 471250, 479250, 487250 }, + khz{ 471250, 479250, 487250 }, // secam-lq - { 615250, 623250, 631250 }, + khz{ 615250, 623250, 631250 }, // pal-m - { 501250, 507250, 513250 }, + khz{ 501250, 507250, 513250 }, // ntsc-m - { 519250, 525250, 531250 }, + khz{ 519250, 525250, 531250 }, // pal-i - { 45750, 53750, 61750 }, + khz{ 45750, 53750, 61750 }, // secam-dk - { 759250, 767250, 775250 }, + khz{ 759250, 767250, 775250 }, // secam-bg - { 495250, 503250, 511250 }, + khz{ 495250, 503250, 511250 }, // secam-l - { 639250, 647250, 655250 }, + khz{ 639250, 647250, 655250 }, // pal-dk - { 783250, 791250, 799250 } + khz{ 783250, 791250, 799250 }, }, // satellite - { + freqs{ // pal-bg - { 519250, 527250, 535250 }, + khz{ 519250, 527250, 535250 }, // secam-lq - { 663250, 671250, 679250 }, + khz{ 663250, 671250, 679250 }, // pal-m - { 537250, 543250, 549250 }, + khz{ 537250, 543250, 549250 }, // ntsc-m - { 555250, 561250, 567250 }, + khz{ 555250, 561250, 567250 }, // pal-i - { 175250, 183250, 191250 }, + khz{ 175250, 183250, 191250 }, // secam-dk - { 807250, 815250, 823250 }, + khz{ 807250, 815250, 823250 }, // secam-bg - { 543250, 551250, 559250 }, + khz{ 543250, 551250, 559250 }, // secam-l - { 687250, 695250, 703250 }, + khz{ 687250, 695250, 703250 }, // pal-dk - { 831250, 839250, 847250 } + khz{ 831250, 839250, 847250 }, }, // terrestrial - { + freqs{ // pal-bg - { 567250, 575250, 583250 }, + khz{ 567250, 575250, 583250 }, // secam-lq - { 711250, 719250, 727250 }, + khz{ 711250, 719250, 727250 }, // pal-m - { 573250, 579250, 585250 }, + khz{ 573250, 579250, 585250 }, // ntsc-m - { 591250, 597250, 603250 }, + khz{ 591250, 597250, 603250 }, // pal-i - { 199250, 207250, 215250 }, + khz{ 199250, 207250, 215250 }, // secam-dk - { 145250, 153250, 161250 }, + khz{ 145250, 153250, 161250 }, // secam-bg - { 591250, 599250, 607250 }, + khz{ 591250, 599250, 607250 }, // secam-l - { 735250, 743250, 751250 }, + khz{ 735250, 743250, 751250 }, // pal-dk - { 169250, 177250, 185250 } - } + khz{ 169250, 177250, 185250 }, + }, }; void tuner_dev_info_init(struct state *state) -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev ` (2 preceding siblings ...) 2021-04-21 7:20 ` [PATCH 4/8] cec-tuner: std::array conversions Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 8:23 ` Hans Verkuil 2021-04-21 7:20 ` [PATCH 6/8] v4l2-utils: turn fb_formats to constexpr array Rosen Penev ` (2 subsequent siblings) 6 siblings, 1 reply; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media The array has a nullptr and 0 member for some reason. Remove and convert loop to a for range one. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp index cb3cb91f7..0359cf137 100644 --- a/utils/common/v4l2-info.cpp +++ b/utils/common/v4l2-info.cpp @@ -3,6 +3,8 @@ * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. */ +#include <array> + #include <v4l2-info.h> static std::string num2s(unsigned num, bool is_hex = true) @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) return flags2s(flags, mbus_ycbcr_def); } -static const flag_def selection_targets_def[] = { - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, - { 0, nullptr } +static constexpr std::array<flag_def, 8> selection_targets_def{ + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, }; bool valid_seltarget_at_idx(unsigned i) { - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; + return i < selection_targets_def.size(); } unsigned seltarget_at_idx(unsigned i) @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) std::string seltarget2s(__u32 target) { - int i = 0; - - while (selection_targets_def[i].str != nullptr) { - if (selection_targets_def[i].flag == target) - return selection_targets_def[i].str; - i++; - } + for (const auto &def : selection_targets_def) + if (def.flag == target) + return def.str; return std::string("Unknown (") + num2s(target) + ")"; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 7:20 ` [PATCH 5/8] v4l2-info: remove a strange sizeof usage Rosen Penev @ 2021-04-21 8:23 ` Hans Verkuil 2021-04-21 9:19 ` Rosen Penev 0 siblings, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2021-04-21 8:23 UTC (permalink / raw) To: Rosen Penev, linux-media On 21/04/2021 09:20, Rosen Penev wrote: > The array has a nullptr and 0 member for some reason. Remove and convert > loop to a for range one. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ > 1 file changed, 15 insertions(+), 18 deletions(-) > > diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp > index cb3cb91f7..0359cf137 100644 > --- a/utils/common/v4l2-info.cpp > +++ b/utils/common/v4l2-info.cpp > @@ -3,6 +3,8 @@ > * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > */ > > +#include <array> > + > #include <v4l2-info.h> > > static std::string num2s(unsigned num, bool is_hex = true) > @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) > return flags2s(flags, mbus_ycbcr_def); > } > > -static const flag_def selection_targets_def[] = { > - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > - { 0, nullptr } The idea of having this sentinel is that this makes it easy to add new entries without having to update the array size. > +static constexpr std::array<flag_def, 8> selection_targets_def{ Something you need to do here, adding a new flag means updating the size. New flags are added regularly, so keeping that robust is a good idea IMHO. If it were possible to write: static constexpr std::array<flag_def> selection_targets_def{ i.e. without an explicit size, then this would make sense, but C++ doesn't allow this. And std::vector allocates the data on the heap, which is less efficient as well. Let's just keep using normal arrays in this case, they do the job just fine. Just because you have a hammer, it doesn't mean everything is now a nail :-) Regards, Hans > + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > }; > > bool valid_seltarget_at_idx(unsigned i) > { > - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; > + return i < selection_targets_def.size(); > } > > unsigned seltarget_at_idx(unsigned i) > @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) > > std::string seltarget2s(__u32 target) > { > - int i = 0; > - > - while (selection_targets_def[i].str != nullptr) { > - if (selection_targets_def[i].flag == target) > - return selection_targets_def[i].str; > - i++; > - } > + for (const auto &def : selection_targets_def) > + if (def.flag == target) > + return def.str; > return std::string("Unknown (") + num2s(target) + ")"; > } > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 8:23 ` Hans Verkuil @ 2021-04-21 9:19 ` Rosen Penev 2021-04-21 9:33 ` Hans Verkuil 0 siblings, 1 reply; 17+ messages in thread From: Rosen Penev @ 2021-04-21 9:19 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media > On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 09:20, Rosen Penev wrote: >> The array has a nullptr and 0 member for some reason. Remove and convert >> loop to a for range one. >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >> 1 file changed, 15 insertions(+), 18 deletions(-) >> >> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >> index cb3cb91f7..0359cf137 100644 >> --- a/utils/common/v4l2-info.cpp >> +++ b/utils/common/v4l2-info.cpp >> @@ -3,6 +3,8 @@ >> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >> */ >> >> +#include <array> >> + >> #include <v4l2-info.h> >> >> static std::string num2s(unsigned num, bool is_hex = true) >> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >> return flags2s(flags, mbus_ycbcr_def); >> } >> >> -static const flag_def selection_targets_def[] = { >> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >> - { 0, nullptr } > > The idea of having this sentinel is that this makes it easy to add new > entries without having to update the array size. Not following this. I assume it’s some C feature. > >> +static constexpr std::array<flag_def, 8> selection_targets_def{ > > Something you need to do here, adding a new flag means updating the size. I assume this is a small issue. It’s an immediate compile error anyway. > > New flags are added regularly, so keeping that robust is a good idea IMHO. > > If it were possible to write: > > static constexpr std::array<flag_def> selection_targets_def{ > > i.e. without an explicit size, then this would make sense, but C++ > doesn't allow this. And std::vector allocates the data on the heap, > which is less efficient as well. But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > Let's just keep using normal arrays in this case, they do the job > just fine. Just because you have a hammer, it doesn't mean everything > is now a nail :-) > > Regards, > > Hans > >> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >> }; >> >> bool valid_seltarget_at_idx(unsigned i) >> { >> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >> + return i < selection_targets_def.size(); >> } >> >> unsigned seltarget_at_idx(unsigned i) >> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >> >> std::string seltarget2s(__u32 target) >> { >> - int i = 0; >> - >> - while (selection_targets_def[i].str != nullptr) { >> - if (selection_targets_def[i].flag == target) >> - return selection_targets_def[i].str; >> - i++; >> - } >> + for (const auto &def : selection_targets_def) >> + if (def.flag == target) >> + return def.str; >> return std::string("Unknown (") + num2s(target) + ")"; >> } >> >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 9:19 ` Rosen Penev @ 2021-04-21 9:33 ` Hans Verkuil 2021-04-21 9:39 ` Rosen Penev 2021-04-21 16:01 ` Nicolas Dufresne 0 siblings, 2 replies; 17+ messages in thread From: Hans Verkuil @ 2021-04-21 9:33 UTC (permalink / raw) To: Rosen Penev; +Cc: linux-media On 21/04/2021 11:19, Rosen Penev wrote: > >> On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: >> >>> On 21/04/2021 09:20, Rosen Penev wrote: >>> The array has a nullptr and 0 member for some reason. Remove and convert >>> loop to a for range one. >>> >>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >>> --- >>> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >>> 1 file changed, 15 insertions(+), 18 deletions(-) >>> >>> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >>> index cb3cb91f7..0359cf137 100644 >>> --- a/utils/common/v4l2-info.cpp >>> +++ b/utils/common/v4l2-info.cpp >>> @@ -3,6 +3,8 @@ >>> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >>> */ >>> >>> +#include <array> >>> + >>> #include <v4l2-info.h> >>> >>> static std::string num2s(unsigned num, bool is_hex = true) >>> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >>> return flags2s(flags, mbus_ycbcr_def); >>> } >>> >>> -static const flag_def selection_targets_def[] = { >>> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>> - { 0, nullptr } >> >> The idea of having this sentinel is that this makes it easy to add new >> entries without having to update the array size. > Not following this. I assume it’s some C feature. Standard programming techique: https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. >> >>> +static constexpr std::array<flag_def, 8> selection_targets_def{ >> >> Something you need to do here, adding a new flag means updating the size. > I assume this is a small issue. It’s an immediate compile error anyway. Not if the size is larger that the number of initializers. That would leave zeroed elements at the end. >> >> New flags are added regularly, so keeping that robust is a good idea IMHO. >> >> If it were possible to write: >> >> static constexpr std::array<flag_def> selection_targets_def{ >> >> i.e. without an explicit size, then this would make sense, but C++ >> doesn't allow this. And std::vector allocates the data on the heap, >> which is less efficient as well. > But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. How is that done in C++17? I didn't find anything about that. Regards, Hans >> >> Let's just keep using normal arrays in this case, they do the job >> just fine. Just because you have a hammer, it doesn't mean everything >> is now a nail :-) >> >> Regards, >> >> Hans >> >>> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>> }; >>> >>> bool valid_seltarget_at_idx(unsigned i) >>> { >>> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >>> + return i < selection_targets_def.size(); >>> } >>> >>> unsigned seltarget_at_idx(unsigned i) >>> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >>> >>> std::string seltarget2s(__u32 target) >>> { >>> - int i = 0; >>> - >>> - while (selection_targets_def[i].str != nullptr) { >>> - if (selection_targets_def[i].flag == target) >>> - return selection_targets_def[i].str; >>> - i++; >>> - } >>> + for (const auto &def : selection_targets_def) >>> + if (def.flag == target) >>> + return def.str; >>> return std::string("Unknown (") + num2s(target) + ")"; >>> } >>> >>> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 9:33 ` Hans Verkuil @ 2021-04-21 9:39 ` Rosen Penev 2021-04-21 16:01 ` Nicolas Dufresne 1 sibling, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 9:39 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media > On Apr 21, 2021, at 02:33, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 11:19, Rosen Penev wrote: >> >>>> On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: >>>> >>>> On 21/04/2021 09:20, Rosen Penev wrote: >>>> The array has a nullptr and 0 member for some reason. Remove and convert >>>> loop to a for range one. >>>> >>>> Signed-off-by: Rosen Penev <rosenp@gmail.com> >>>> --- >>>> utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ >>>> 1 file changed, 15 insertions(+), 18 deletions(-) >>>> >>>> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp >>>> index cb3cb91f7..0359cf137 100644 >>>> --- a/utils/common/v4l2-info.cpp >>>> +++ b/utils/common/v4l2-info.cpp >>>> @@ -3,6 +3,8 @@ >>>> * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. >>>> */ >>>> >>>> +#include <array> >>>> + >>>> #include <v4l2-info.h> >>>> >>>> static std::string num2s(unsigned num, bool is_hex = true) >>>> @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) >>>> return flags2s(flags, mbus_ycbcr_def); >>>> } >>>> >>>> -static const flag_def selection_targets_def[] = { >>>> - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>>> - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>>> - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>>> - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>>> - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>>> - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>>> - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>>> - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>>> - { 0, nullptr } >>> >>> The idea of having this sentinel is that this makes it easy to add new >>> entries without having to update the array size. >> Not following this. I assume it’s some C feature. > > Standard programming techique: > > https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. > >>> >>>> +static constexpr std::array<flag_def, 8> selection_targets_def{ >>> >>> Something you need to do here, adding a new flag means updating the size. >> I assume this is a small issue. It’s an immediate compile error anyway. > > Not if the size is larger that the number of initializers. That would leave > zeroed elements at the end. Ah yes that is a real problem. I’ve been bitten by it in the past. > >>> >>> New flags are added regularly, so keeping that robust is a good idea IMHO. >>> >>> If it were possible to write: >>> >>> static constexpr std::array<flag_def> selection_targets_def{ >>> >>> i.e. without an explicit size, then this would make sense, but C++ >>> doesn't allow this. And std::vector allocates the data on the heap, >>> which is less efficient as well. >> But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > How is that done in C++17? I didn't find anything about that. C++17 has class template argument deduction. It allows putting just std::array or std::vector or any other container really without the <>. Of course you have to allow it to deduce the arguments by putting them in the initializers. This is actually what I did in the previous cec-tuner patch. > > Regards, > > Hans > >>> >>> Let's just keep using normal arrays in this case, they do the job >>> just fine. Just because you have a hammer, it doesn't mean everything >>> is now a nail :-) >>> >>> Regards, >>> >>> Hans >>> >>>> + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, >>>> + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, >>>> + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, >>>> + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, >>>> + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, >>>> }; >>>> >>>> bool valid_seltarget_at_idx(unsigned i) >>>> { >>>> - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; >>>> + return i < selection_targets_def.size(); >>>> } >>>> >>>> unsigned seltarget_at_idx(unsigned i) >>>> @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) >>>> >>>> std::string seltarget2s(__u32 target) >>>> { >>>> - int i = 0; >>>> - >>>> - while (selection_targets_def[i].str != nullptr) { >>>> - if (selection_targets_def[i].flag == target) >>>> - return selection_targets_def[i].str; >>>> - i++; >>>> - } >>>> + for (const auto &def : selection_targets_def) >>>> + if (def.flag == target) >>>> + return def.str; >>>> return std::string("Unknown (") + num2s(target) + ")"; >>>> } >>>> >>>> >>> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/8] v4l2-info: remove a strange sizeof usage 2021-04-21 9:33 ` Hans Verkuil 2021-04-21 9:39 ` Rosen Penev @ 2021-04-21 16:01 ` Nicolas Dufresne 1 sibling, 0 replies; 17+ messages in thread From: Nicolas Dufresne @ 2021-04-21 16:01 UTC (permalink / raw) To: Hans Verkuil, Rosen Penev; +Cc: linux-media Le mercredi 21 avril 2021 à 11:33 +0200, Hans Verkuil a écrit : > On 21/04/2021 11:19, Rosen Penev wrote: > > > > > On Apr 21, 2021, at 01:23, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > > > > > > On 21/04/2021 09:20, Rosen Penev wrote: > > > > The array has a nullptr and 0 member for some reason. Remove and convert > > > > loop to a for range one. > > > > > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > > > --- > > > > utils/common/v4l2-info.cpp | 33 +++++++++++++++------------------ > > > > 1 file changed, 15 insertions(+), 18 deletions(-) > > > > > > > > diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp > > > > index cb3cb91f7..0359cf137 100644 > > > > --- a/utils/common/v4l2-info.cpp > > > > +++ b/utils/common/v4l2-info.cpp > > > > @@ -3,6 +3,8 @@ > > > > * Copyright 2018 Cisco Systems, Inc. and/or its affiliates. All rights reserved. > > > > */ > > > > > > > > +#include <array> > > > > + > > > > #include <v4l2-info.h> > > > > > > > > static std::string num2s(unsigned num, bool is_hex = true) > > > > @@ -411,21 +413,20 @@ std::string mbus2s(unsigned flags, bool is_hsv) > > > > return flags2s(flags, mbus_ycbcr_def); > > > > } > > > > > > > > -static const flag_def selection_targets_def[] = { > > > > - { V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > > > > - { V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > > > > - { V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > > > > - { V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > > > > - { V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > > > > - { V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > > > > - { V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > > > > - { V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > > > > - { 0, nullptr } > > > > > > The idea of having this sentinel is that this makes it easy to add new > > > entries without having to update the array size. > > Not following this. I assume it’s some C feature. > > Standard programming techique: > > https://en.wikipedia.org/wiki/Sentinel_value#:~:text=In%20computer%20programming%2C%20a%20sentinel,a%20loop%20or%20recursive%20algorithm. > > > > > > > > +static constexpr std::array<flag_def, 8> selection_targets_def{ > > > > > > Something you need to do here, adding a new flag means updating the size. > > I assume this is a small issue. It’s an immediate compile error anyway. > > Not if the size is larger that the number of initializers. That would leave > zeroed elements at the end. > > > > > > > New flags are added regularly, so keeping that robust is a good idea IMHO. > > > > > > If it were possible to write: > > > > > > static constexpr std::array<flag_def> selection_targets_def{ > > > > > > i.e. without an explicit size, then this would make sense, but C++ > > > doesn't allow this. And std::vector allocates the data on the heap, > > > which is less efficient as well. > > But this is possible. With C++17 :). That would necessitate a minimum of GCC6 though. > > How is that done in C++17? I didn't find anything about that. I believe this is similar to this code: https://git.linuxtv.org/libcamera.git/tree/src/gstreamer/gstlibcamera-utils.cpp#n15 But there I only used the fact that you can iterate it like any other collection. This is a recent feature I got told (I'm quite a C++ newby, and I like static arrays without passing and updating a size). > > Regards, > > Hans > > > > > > > Let's just keep using normal arrays in this case, they do the job > > > just fine. Just because you have a hammer, it doesn't mean everything > > > is now a nail :-) > > > > > > Regards, > > > > > > Hans > > > > > > > + flag_def{ V4L2_SEL_TGT_CROP_ACTIVE, "crop" }, > > > > + flag_def{ V4L2_SEL_TGT_CROP_DEFAULT, "crop_default" }, > > > > + flag_def{ V4L2_SEL_TGT_CROP_BOUNDS, "crop_bounds" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_ACTIVE, "compose" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_DEFAULT, "compose_default" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_BOUNDS, "compose_bounds" }, > > > > + flag_def{ V4L2_SEL_TGT_COMPOSE_PADDED, "compose_padded" }, > > > > + flag_def{ V4L2_SEL_TGT_NATIVE_SIZE, "native_size" }, > > > > }; > > > > > > > > bool valid_seltarget_at_idx(unsigned i) > > > > { > > > > - return i < sizeof(selection_targets_def) / sizeof(selection_targets_def[0]) - 1; > > > > + return i < selection_targets_def.size(); > > > > } > > > > > > > > unsigned seltarget_at_idx(unsigned i) > > > > @@ -437,13 +438,9 @@ unsigned seltarget_at_idx(unsigned i) > > > > > > > > std::string seltarget2s(__u32 target) > > > > { > > > > - int i = 0; > > > > - > > > > - while (selection_targets_def[i].str != nullptr) { > > > > - if (selection_targets_def[i].flag == target) > > > > - return selection_targets_def[i].str; > > > > - i++; > > > > - } > > > > + for (const auto &def : selection_targets_def) > > > > + if (def.flag == target) > > > > + return def.str; > > > > return std::string("Unknown (") + num2s(target) + ")"; > > > > } > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/8] v4l2-utils: turn fb_formats to constexpr array 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev ` (3 preceding siblings ...) 2021-04-21 7:20 ` [PATCH 5/8] v4l2-info: remove a strange sizeof usage Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 7:20 ` [PATCH 7/8] v4l2-utils: turn mbus_names into const vector Rosen Penev 2021-04-21 7:20 ` [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array Rosen Penev 6 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media Forces evaluation at compile time and allows usage of a for range loop. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/v4l2-ctl/v4l2-ctl-overlay.cpp | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/utils/v4l2-ctl/v4l2-ctl-overlay.cpp b/utils/v4l2-ctl/v4l2-ctl-overlay.cpp index 09de70c85..639a41757 100644 --- a/utils/v4l2-ctl/v4l2-ctl-overlay.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-overlay.cpp @@ -1,3 +1,4 @@ +#include <array> #include <vector> #include <linux/fb.h> @@ -121,16 +122,15 @@ struct bitfield2fmt { __u32 pixfmt; }; -static const struct bitfield2fmt fb_formats[] = { - { 10, 5, 5, 5, 0, 5, 15, 1, V4L2_PIX_FMT_ARGB555 }, - { 11, 5, 5, 6, 0, 5, 0, 0, V4L2_PIX_FMT_RGB565 }, - { 1, 5, 6, 5, 11, 5, 0, 1, V4L2_PIX_FMT_RGB555X }, - { 0, 5, 5, 6, 11, 5, 0, 0, V4L2_PIX_FMT_RGB565X }, - { 16, 8, 8, 8, 0, 8, 0, 0, V4L2_PIX_FMT_BGR24 }, - { 0, 8, 8, 8, 16, 8, 0, 0, V4L2_PIX_FMT_RGB24 }, - { 16, 8, 8, 8, 0, 8, 24, 8, V4L2_PIX_FMT_ABGR32 }, - { 8, 8, 16, 8, 24, 8, 0, 8, V4L2_PIX_FMT_ARGB32 }, - { } +static constexpr std::array<bitfield2fmt, 8> fb_formats{ + bitfield2fmt{ 10, 5, 5, 5, 0, 5, 15, 1, V4L2_PIX_FMT_ARGB555 }, + bitfield2fmt{ 11, 5, 5, 6, 0, 5, 0, 0, V4L2_PIX_FMT_RGB565 }, + bitfield2fmt{ 1, 5, 6, 5, 11, 5, 0, 1, V4L2_PIX_FMT_RGB555X }, + bitfield2fmt{ 0, 5, 5, 6, 11, 5, 0, 0, V4L2_PIX_FMT_RGB565X }, + bitfield2fmt{ 16, 8, 8, 8, 0, 8, 0, 0, V4L2_PIX_FMT_BGR24 }, + bitfield2fmt{ 0, 8, 8, 8, 16, 8, 0, 0, V4L2_PIX_FMT_RGB24 }, + bitfield2fmt{ 16, 8, 8, 8, 0, 8, 24, 8, V4L2_PIX_FMT_ABGR32 }, + bitfield2fmt{ 8, 8, 16, 8, 24, 8, 0, 8, V4L2_PIX_FMT_ARGB32 }, }; static bool match_bitfield(const struct fb_bitfield &bf, unsigned off, unsigned len) @@ -182,9 +182,7 @@ static int fbuf_fill_from_fb(struct v4l2_framebuffer &fb, const char *fb_device) if (vi.bits_per_pixel == 8) fb.fmt.pixelformat = V4L2_PIX_FMT_GREY; } else { - for (int i = 0; fb_formats[i].pixfmt; i++) { - const struct bitfield2fmt &p = fb_formats[i]; - + for (const auto &p : fb_formats) { if (match_bitfield(vi.blue, p.blue_off, p.blue_len) && match_bitfield(vi.green, p.green_off, p.green_len) && match_bitfield(vi.red, p.red_off, p.red_len) && -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 7/8] v4l2-utils: turn mbus_names into const vector 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev ` (4 preceding siblings ...) 2021-04-21 7:20 ` [PATCH 6/8] v4l2-utils: turn fb_formats to constexpr array Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 8:02 ` Hans Verkuil 2021-04-21 7:20 ` [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array Rosen Penev 6 siblings, 1 reply; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media Allows usage of a more standard for loop. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/v4l2-ctl/v4l2-ctl-subdev.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp index d0f391bba..f2be9442c 100644 --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp @@ -1,3 +1,5 @@ +#include <vector> + #include "v4l2-ctl.h" struct mbus_name { @@ -5,10 +7,9 @@ struct mbus_name { __u32 code; }; -static struct mbus_name mbus_names[] = { +static const std::vector<mbus_name> mbus_names{ { "Fixed", MEDIA_BUS_FMT_FIXED }, #include "media-bus-format-names.h" - { nullptr, 0 } }; /* selection specified */ @@ -343,9 +344,9 @@ static void print_framefmt(const struct v4l2_mbus_framefmt &fmt) { __u32 colsp = fmt.colorspace; __u32 ycbcr_enc = fmt.ycbcr_enc; - unsigned int i; + size_t i; - for (i = 0; mbus_names[i].name; i++) + for (i = 0; i < mbus_names.size(); i++) if (mbus_names[i].code == fmt.code) break; printf("\tWidth/Height : %u/%u\n", fmt.width, fmt.height); @@ -554,9 +555,9 @@ void subdev_get(cv4l_fd &_fd) static void print_mbus_code(__u32 code) { - unsigned int i; + size_t i; - for (i = 0; mbus_names[i].name; i++) + for (i = 0; i < mbus_names.size(); i++) if (mbus_names[i].code == code) break; if (mbus_names[i].name) @@ -568,9 +569,8 @@ static void print_mbus_code(__u32 code) static void print_mbus_codes(int fd, __u32 pad) { - struct v4l2_subdev_mbus_code_enum mbus_code; + struct v4l2_subdev_mbus_code_enum mbus_code = {}; - memset(&mbus_code, 0, sizeof(mbus_code)); mbus_code.pad = pad; mbus_code.which = V4L2_SUBDEV_FORMAT_TRY; -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 7/8] v4l2-utils: turn mbus_names into const vector 2021-04-21 7:20 ` [PATCH 7/8] v4l2-utils: turn mbus_names into const vector Rosen Penev @ 2021-04-21 8:02 ` Hans Verkuil 2021-04-21 9:25 ` Rosen Penev 0 siblings, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2021-04-21 8:02 UTC (permalink / raw) To: Rosen Penev, linux-media On 21/04/2021 09:20, Rosen Penev wrote: > Allows usage of a more standard for loop. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > utils/v4l2-ctl/v4l2-ctl-subdev.cpp | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp > index d0f391bba..f2be9442c 100644 > --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp > +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp > @@ -1,3 +1,5 @@ > +#include <vector> > + > #include "v4l2-ctl.h" > > struct mbus_name { > @@ -5,10 +7,9 @@ struct mbus_name { > __u32 code; > }; > > -static struct mbus_name mbus_names[] = { > +static const std::vector<mbus_name> mbus_names{ > { "Fixed", MEDIA_BUS_FMT_FIXED }, > #include "media-bus-format-names.h" > - { nullptr, 0 } > }; I see no reason for this change, there is nothing wrong with the current array that I can see (other than that it should have been const, but I'll add that). > > /* selection specified */ > @@ -343,9 +344,9 @@ static void print_framefmt(const struct v4l2_mbus_framefmt &fmt) > { > __u32 colsp = fmt.colorspace; > __u32 ycbcr_enc = fmt.ycbcr_enc; > - unsigned int i; > + size_t i; > > - for (i = 0; mbus_names[i].name; i++) > + for (i = 0; i < mbus_names.size(); i++) > if (mbus_names[i].code == fmt.code) > break; > printf("\tWidth/Height : %u/%u\n", fmt.width, fmt.height); > @@ -554,9 +555,9 @@ void subdev_get(cv4l_fd &_fd) > > static void print_mbus_code(__u32 code) > { > - unsigned int i; > + size_t i; > > - for (i = 0; mbus_names[i].name; i++) > + for (i = 0; i < mbus_names.size(); i++) > if (mbus_names[i].code == code) > break; > if (mbus_names[i].name) > @@ -568,9 +569,8 @@ static void print_mbus_code(__u32 code) > > static void print_mbus_codes(int fd, __u32 pad) > { > - struct v4l2_subdev_mbus_code_enum mbus_code; > + struct v4l2_subdev_mbus_code_enum mbus_code = {}; > > - memset(&mbus_code, 0, sizeof(mbus_code)); This one is nice to have, though. But it's independent of the other changes. Regards, Hans > mbus_code.pad = pad; > mbus_code.which = V4L2_SUBDEV_FORMAT_TRY; > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 7/8] v4l2-utils: turn mbus_names into const vector 2021-04-21 8:02 ` Hans Verkuil @ 2021-04-21 9:25 ` Rosen Penev 0 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 9:25 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media > On Apr 21, 2021, at 01:02, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 09:20, Rosen Penev wrote: >> Allows usage of a more standard for loop. >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> utils/v4l2-ctl/v4l2-ctl-subdev.cpp | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp >> index d0f391bba..f2be9442c 100644 >> --- a/utils/v4l2-ctl/v4l2-ctl-subdev.cpp >> +++ b/utils/v4l2-ctl/v4l2-ctl-subdev.cpp >> @@ -1,3 +1,5 @@ >> +#include <vector> >> + >> #include "v4l2-ctl.h" >> >> struct mbus_name { >> @@ -5,10 +7,9 @@ struct mbus_name { >> __u32 code; >> }; >> >> -static struct mbus_name mbus_names[] = { >> +static const std::vector<mbus_name> mbus_names{ >> { "Fixed", MEDIA_BUS_FMT_FIXED }, >> #include "media-bus-format-names.h" >> - { nullptr, 0 } >> }; > > I see no reason for this change, there is nothing wrong with the > current array that I can see (other than that it should have been > const, but I'll add that). I used vector since it’s not easy to see the size. > >> >> /* selection specified */ >> @@ -343,9 +344,9 @@ static void print_framefmt(const struct v4l2_mbus_framefmt &fmt) >> { >> __u32 colsp = fmt.colorspace; >> __u32 ycbcr_enc = fmt.ycbcr_enc; >> - unsigned int i; >> + size_t i; >> >> - for (i = 0; mbus_names[i].name; i++) >> + for (i = 0; i < mbus_names.size(); i++) >> if (mbus_names[i].code == fmt.code) >> break; >> printf("\tWidth/Height : %u/%u\n", fmt.width, fmt.height); >> @@ -554,9 +555,9 @@ void subdev_get(cv4l_fd &_fd) >> >> static void print_mbus_code(__u32 code) >> { >> - unsigned int i; >> + size_t i; >> >> - for (i = 0; mbus_names[i].name; i++) >> + for (i = 0; i < mbus_names.size(); i++) >> if (mbus_names[i].code == code) >> break; >> if (mbus_names[i].name) >> @@ -568,9 +569,8 @@ static void print_mbus_code(__u32 code) >> >> static void print_mbus_codes(int fd, __u32 pad) >> { >> - struct v4l2_subdev_mbus_code_enum mbus_code; >> + struct v4l2_subdev_mbus_code_enum mbus_code = {}; >> >> - memset(&mbus_code, 0, sizeof(mbus_code)); > > This one is nice to have, though. But it's independent of the other > changes. Noticed it as I was passing by :). > > Regards, > > Hans > > >> mbus_code.pad = pad; >> mbus_code.which = V4L2_SUBDEV_FORMAT_TRY; >> >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev ` (5 preceding siblings ...) 2021-04-21 7:20 ` [PATCH 7/8] v4l2-utils: turn mbus_names into const vector Rosen Penev @ 2021-04-21 7:20 ` Rosen Penev 2021-04-21 8:06 ` Hans Verkuil 6 siblings, 1 reply; 17+ messages in thread From: Rosen Penev @ 2021-04-21 7:20 UTC (permalink / raw) To: linux-media Allows usage of a single any_of instead of a raw loop. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- utils/v4l2-ctl/v4l2-ctl-common.cpp | 24 +++++------------------- 1 file changed, 5 insertions(+), 19 deletions(-) diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp index 17ad488dd..2b6dd6d13 100644 --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp @@ -116,28 +116,14 @@ void common_usage() ); } -static const char *prefixes[] = { - "video", - "radio", - "vbi", - "swradio", - "v4l-subdev", - "v4l-touch", - "media", - nullptr +static constexpr std::array<const char *, 7> prefixes{ + "video", "radio", "vbi", "swradio", "v4l-subdev", "v4l-touch", "media", }; static bool is_v4l_dev(const char *name) { - for (unsigned i = 0; prefixes[i]; i++) { - unsigned l = strlen(prefixes[i]); - - if (!memcmp(name, prefixes[i], l)) { - if (isdigit(name[l])) - return true; - } - } - return false; + return std::any_of(prefixes.begin(), prefixes.end(), + [=](const char *prefix) { return !strcmp(name, prefix) && isdigit(name[strlen(prefix)]); }); } static int calc_node_val(const char *s) @@ -146,7 +132,7 @@ static int calc_node_val(const char *s) s = std::strrchr(s, '/') + 1; - for (unsigned i = 0; prefixes[i]; i++) { + for (size_t i = 0; i < prefixes.size(); i++) { unsigned l = strlen(prefixes[i]); if (!memcmp(s, prefixes[i], l)) { -- 2.30.2 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array 2021-04-21 7:20 ` [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array Rosen Penev @ 2021-04-21 8:06 ` Hans Verkuil 2021-04-21 9:23 ` Rosen Penev 0 siblings, 1 reply; 17+ messages in thread From: Hans Verkuil @ 2021-04-21 8:06 UTC (permalink / raw) To: Rosen Penev, linux-media On 21/04/2021 09:20, Rosen Penev wrote: > Allows usage of a single any_of instead of a raw loop. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > utils/v4l2-ctl/v4l2-ctl-common.cpp | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp > index 17ad488dd..2b6dd6d13 100644 > --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp > +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp > @@ -116,28 +116,14 @@ void common_usage() > ); > } > > -static const char *prefixes[] = { > - "video", > - "radio", > - "vbi", > - "swradio", > - "v4l-subdev", > - "v4l-touch", > - "media", > - nullptr > +static constexpr std::array<const char *, 7> prefixes{ > + "video", "radio", "vbi", "swradio", "v4l-subdev", "v4l-touch", "media", > }; > > static bool is_v4l_dev(const char *name) > { > - for (unsigned i = 0; prefixes[i]; i++) { > - unsigned l = strlen(prefixes[i]); > - > - if (!memcmp(name, prefixes[i], l)) { > - if (isdigit(name[l])) > - return true; > - } > - } > - return false; > + return std::any_of(prefixes.begin(), prefixes.end(), > + [=](const char *prefix) { return !strcmp(name, prefix) && isdigit(name[strlen(prefix)]); }); Yuck. Besides, it is wrong AFAIKS since strcmp is not the same as memcmp. I like the original code, easier to understand than the replacement. So sue me :-) Regards, Hans > } > > static int calc_node_val(const char *s) > @@ -146,7 +132,7 @@ static int calc_node_val(const char *s) > > s = std::strrchr(s, '/') + 1; > > - for (unsigned i = 0; prefixes[i]; i++) { > + for (size_t i = 0; i < prefixes.size(); i++) { > unsigned l = strlen(prefixes[i]); > > if (!memcmp(s, prefixes[i], l)) { > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array 2021-04-21 8:06 ` Hans Verkuil @ 2021-04-21 9:23 ` Rosen Penev 0 siblings, 0 replies; 17+ messages in thread From: Rosen Penev @ 2021-04-21 9:23 UTC (permalink / raw) To: Hans Verkuil; +Cc: linux-media > On Apr 21, 2021, at 01:06, Hans Verkuil <hverkuil@xs4all.nl> wrote: > >> On 21/04/2021 09:20, Rosen Penev wrote: >> Allows usage of a single any_of instead of a raw loop. >> >> Signed-off-by: Rosen Penev <rosenp@gmail.com> >> --- >> utils/v4l2-ctl/v4l2-ctl-common.cpp | 24 +++++------------------- >> 1 file changed, 5 insertions(+), 19 deletions(-) >> >> diff --git a/utils/v4l2-ctl/v4l2-ctl-common.cpp b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> index 17ad488dd..2b6dd6d13 100644 >> --- a/utils/v4l2-ctl/v4l2-ctl-common.cpp >> +++ b/utils/v4l2-ctl/v4l2-ctl-common.cpp >> @@ -116,28 +116,14 @@ void common_usage() >> ); >> } >> >> -static const char *prefixes[] = { >> - "video", >> - "radio", >> - "vbi", >> - "swradio", >> - "v4l-subdev", >> - "v4l-touch", >> - "media", >> - nullptr >> +static constexpr std::array<const char *, 7> prefixes{ >> + "video", "radio", "vbi", "swradio", "v4l-subdev", "v4l-touch", "media", >> }; >> >> static bool is_v4l_dev(const char *name) >> { >> - for (unsigned i = 0; prefixes[i]; i++) { >> - unsigned l = strlen(prefixes[i]); >> - >> - if (!memcmp(name, prefixes[i], l)) { >> - if (isdigit(name[l])) >> - return true; >> - } >> - } >> - return false; >> + return std::any_of(prefixes.begin(), prefixes.end(), >> + [=](const char *prefix) { return !strcmp(name, prefix) && isdigit(name[strlen(prefix)]); }); > > Yuck. Besides, it is wrong AFAIKS since strcmp is not the same as memcmp. True. C++17 would make this better with std::string_view with its size member function which also happens to be constexpr. strlen IIRC is slow. > > I like the original code, easier to understand than the replacement. So sue me :-) I’ll convert to range loop. > > Regards, > > Hans > >> } >> >> static int calc_node_val(const char *s) >> @@ -146,7 +132,7 @@ static int calc_node_val(const char *s) >> >> s = std::strrchr(s, '/') + 1; >> >> - for (unsigned i = 0; prefixes[i]; i++) { >> + for (size_t i = 0; i < prefixes.size(); i++) { >> unsigned l = strlen(prefixes[i]); >> >> if (!memcmp(s, prefixes[i], l)) { >> > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-04-21 16:01 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-21 7:20 [PATCH 1/8] clang-tidy: use auto Rosen Penev 2021-04-21 7:20 ` [PATCH 2/8] clang-tidy: use nullptr Rosen Penev 2021-04-21 7:20 ` [PATCH 3/8] remove unused ARRAY_SIZE Rosen Penev 2021-04-21 7:20 ` [PATCH 4/8] cec-tuner: std::array conversions Rosen Penev 2021-04-21 7:20 ` [PATCH 5/8] v4l2-info: remove a strange sizeof usage Rosen Penev 2021-04-21 8:23 ` Hans Verkuil 2021-04-21 9:19 ` Rosen Penev 2021-04-21 9:33 ` Hans Verkuil 2021-04-21 9:39 ` Rosen Penev 2021-04-21 16:01 ` Nicolas Dufresne 2021-04-21 7:20 ` [PATCH 6/8] v4l2-utils: turn fb_formats to constexpr array Rosen Penev 2021-04-21 7:20 ` [PATCH 7/8] v4l2-utils: turn mbus_names into const vector Rosen Penev 2021-04-21 8:02 ` Hans Verkuil 2021-04-21 9:25 ` Rosen Penev 2021-04-21 7:20 ` [PATCH 8/8] v4l2-utils: turn prefixes to a constexpr array Rosen Penev 2021-04-21 8:06 ` Hans Verkuil 2021-04-21 9:23 ` Rosen Penev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).