2022-11-18 10:11:14

by Umang Jain

Subject: [PATCH v2 0/3] staging: vc04_services: Promote bool usage

In commit 7967656ffbfa ("coding-style: Clarify the expectations around
bool") the check to dis-allow bool structure members was removed from
checkpatch.pl. It promotes bool structure members to store boolean
values. This enhances code readability.

Changes in v2:
- Drop ':1' in bool foo:1 usage as mistaken understood as initization
by me (apologies).


Umang Jain (3):
Revert "staging: mmal-vchiq: Avoid use of bool in structures"
vc04_services: mmal-vchiq: Use bool for vchiq_mmal_component.in_use
vc04_services: bcm2835-camera: Use bool values for
mmal_fmt.remove_padding

.../bcm2835-camera/bcm2835-camera.c | 30 +++++++++----------
.../vc04_services/vchiq-mmal/mmal-vchiq.c | 18 +++++------
.../vc04_services/vchiq-mmal/mmal-vchiq.h | 6 ++--
3 files changed, 27 insertions(+), 27 deletions(-)

--
2.38.1

2022-11-18 10:13:45

by Umang Jain

Subject: [PATCH v2 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding

mmal_fmt.remove_padding is defined as a boolean type hence, use boolean
values for it instead of 0/1 integers. This enhances code readability.

Signed-off-by: Umang Jain <[email protected]>
---
.../bcm2835-camera/bcm2835-camera.c | 30 +++++++++----------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index fd456d1f7061..797ebe2a973a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -87,21 +87,21 @@ static struct mmal_fmt formats[] = {
.depth = 12,
.mmal_component = COMP_CAMERA,
.ybbp = 1,
- .remove_padding = 1,
+ .remove_padding = true,
}, {
.fourcc = V4L2_PIX_FMT_YUYV,
.mmal = MMAL_ENCODING_YUYV,
.depth = 16,
.mmal_component = COMP_CAMERA,
.ybbp = 2,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_RGB24,
.mmal = MMAL_ENCODING_RGB24,
.depth = 24,
.mmal_component = COMP_CAMERA,
.ybbp = 3,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_JPEG,
.flags = V4L2_FMT_FLAG_COMPRESSED,
@@ -109,7 +109,7 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = COMP_IMAGE_ENCODE,
.ybbp = 0,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_H264,
.flags = V4L2_FMT_FLAG_COMPRESSED,
@@ -117,7 +117,7 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = COMP_VIDEO_ENCODE,
.ybbp = 0,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_MJPEG,
.flags = V4L2_FMT_FLAG_COMPRESSED,
@@ -125,63 +125,63 @@ static struct mmal_fmt formats[] = {
.depth = 8,
.mmal_component = COMP_VIDEO_ENCODE,
.ybbp = 0,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_YVYU,
.mmal = MMAL_ENCODING_YVYU,
.depth = 16,
.mmal_component = COMP_CAMERA,
.ybbp = 2,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_VYUY,
.mmal = MMAL_ENCODING_VYUY,
.depth = 16,
.mmal_component = COMP_CAMERA,
.ybbp = 2,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_UYVY,
.mmal = MMAL_ENCODING_UYVY,
.depth = 16,
.mmal_component = COMP_CAMERA,
.ybbp = 2,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_NV12,
.mmal = MMAL_ENCODING_NV12,
.depth = 12,
.mmal_component = COMP_CAMERA,
.ybbp = 1,
- .remove_padding = 1,
+ .remove_padding = true,
}, {
.fourcc = V4L2_PIX_FMT_BGR24,
.mmal = MMAL_ENCODING_BGR24,
.depth = 24,
.mmal_component = COMP_CAMERA,
.ybbp = 3,
- .remove_padding = 0,
+ .remove_padding = false,
}, {
.fourcc = V4L2_PIX_FMT_YVU420,
.mmal = MMAL_ENCODING_YV12,
.depth = 12,
.mmal_component = COMP_CAMERA,
.ybbp = 1,
- .remove_padding = 1,
+ .remove_padding = true,
}, {
.fourcc = V4L2_PIX_FMT_NV21,
.mmal = MMAL_ENCODING_NV21,
.depth = 12,
.mmal_component = COMP_CAMERA,
.ybbp = 1,
- .remove_padding = 1,
+ .remove_padding = true,
}, {
.fourcc = V4L2_PIX_FMT_BGR32,
.mmal = MMAL_ENCODING_BGRA,
.depth = 32,
.mmal_component = COMP_CAMERA,
.ybbp = 4,
- .remove_padding = 0,
+ .remove_padding = false,
},
};

@@ -1147,7 +1147,7 @@ static int mmal_setup_components(struct bcm2835_mmal_dev *dev,
struct vchiq_mmal_port *port = NULL, *camera_port = NULL;
struct vchiq_mmal_component *encode_component = NULL;
struct mmal_fmt *mfmt = get_format(f);
- u32 remove_padding;
+ bool remove_padding;

if (!mfmt)
return -EINVAL;
--
2.38.1

2022-11-18 11:22:28

by Kieran Bingham

Subject: Re: [PATCH v2 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding

Quoting Umang Jain (2022-11-18 08:42:44)
> mmal_fmt.remove_padding is defined as a boolean type hence, use boolean
> values for it instead of 0/1 integers. This enhances code readability.

This could be (optionally) expanded:

"""
The field 'remove_padding' in 'struct mmal_fmt' is defined as a boolean
type.

The initialisation of the formats[] array stores integer 0/1 values into
this field rather than a more readable bool type.

Convert the usages to boolean true/false, and fix up the local storage
type in mmal_setup_components() to enhance code readability.
"""

Reviewed-by: Kieran Bingham <[email protected]>


> Signed-off-by: Umang Jain <[email protected]>
> ---
> .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++----------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index fd456d1f7061..797ebe2a973a 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -87,21 +87,21 @@ static struct mmal_fmt formats[] = {
> .depth = 12,
> .mmal_component = COMP_CAMERA,
> .ybbp = 1,
> - .remove_padding = 1,
> + .remove_padding = true,
> }, {
> .fourcc = V4L2_PIX_FMT_YUYV,
> .mmal = MMAL_ENCODING_YUYV,
> .depth = 16,
> .mmal_component = COMP_CAMERA,
> .ybbp = 2,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_RGB24,
> .mmal = MMAL_ENCODING_RGB24,
> .depth = 24,
> .mmal_component = COMP_CAMERA,
> .ybbp = 3,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_JPEG,
> .flags = V4L2_FMT_FLAG_COMPRESSED,
> @@ -109,7 +109,7 @@ static struct mmal_fmt formats[] = {
> .depth = 8,
> .mmal_component = COMP_IMAGE_ENCODE,
> .ybbp = 0,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_H264,
> .flags = V4L2_FMT_FLAG_COMPRESSED,
> @@ -117,7 +117,7 @@ static struct mmal_fmt formats[] = {
> .depth = 8,
> .mmal_component = COMP_VIDEO_ENCODE,
> .ybbp = 0,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_MJPEG,
> .flags = V4L2_FMT_FLAG_COMPRESSED,
> @@ -125,63 +125,63 @@ static struct mmal_fmt formats[] = {
> .depth = 8,
> .mmal_component = COMP_VIDEO_ENCODE,
> .ybbp = 0,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_YVYU,
> .mmal = MMAL_ENCODING_YVYU,
> .depth = 16,
> .mmal_component = COMP_CAMERA,
> .ybbp = 2,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_VYUY,
> .mmal = MMAL_ENCODING_VYUY,
> .depth = 16,
> .mmal_component = COMP_CAMERA,
> .ybbp = 2,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_UYVY,
> .mmal = MMAL_ENCODING_UYVY,
> .depth = 16,
> .mmal_component = COMP_CAMERA,
> .ybbp = 2,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_NV12,
> .mmal = MMAL_ENCODING_NV12,
> .depth = 12,
> .mmal_component = COMP_CAMERA,
> .ybbp = 1,
> - .remove_padding = 1,
> + .remove_padding = true,
> }, {
> .fourcc = V4L2_PIX_FMT_BGR24,
> .mmal = MMAL_ENCODING_BGR24,
> .depth = 24,
> .mmal_component = COMP_CAMERA,
> .ybbp = 3,
> - .remove_padding = 0,
> + .remove_padding = false,
> }, {
> .fourcc = V4L2_PIX_FMT_YVU420,
> .mmal = MMAL_ENCODING_YV12,
> .depth = 12,
> .mmal_component = COMP_CAMERA,
> .ybbp = 1,
> - .remove_padding = 1,
> + .remove_padding = true,
> }, {
> .fourcc = V4L2_PIX_FMT_NV21,
> .mmal = MMAL_ENCODING_NV21,
> .depth = 12,
> .mmal_component = COMP_CAMERA,
> .ybbp = 1,
> - .remove_padding = 1,
> + .remove_padding = true,
> }, {
> .fourcc = V4L2_PIX_FMT_BGR32,
> .mmal = MMAL_ENCODING_BGRA,
> .depth = 32,
> .mmal_component = COMP_CAMERA,
> .ybbp = 4,
> - .remove_padding = 0,
> + .remove_padding = false,
> },
> };
>
> @@ -1147,7 +1147,7 @@ static int mmal_setup_components(struct bcm2835_mmal_dev *dev,
> struct vchiq_mmal_port *port = NULL, *camera_port = NULL;
> struct vchiq_mmal_component *encode_component = NULL;
> struct mmal_fmt *mfmt = get_format(f);
> - u32 remove_padding;
> + bool remove_padding;
>
> if (!mfmt)
> return -EINVAL;
> --
> 2.38.1
>

2022-11-18 17:32:01

by Dave Stevenson

Subject: Re: [PATCH v2 3/3] vc04_services: bcm2835-camera: Use bool values for mmal_fmt.remove_padding

On Fri, 18 Nov 2022 at 10:50, Kieran Bingham
<[email protected]> wrote:
>
> Quoting Umang Jain (2022-11-18 08:42:44)
> > mmal_fmt.remove_padding is defined as a boolean type hence, use boolean
> > values for it instead of 0/1 integers. This enhances code readability.
>
> This could be (optionally) expanded:
>
> """
> The field 'remove_padding' in 'struct mmal_fmt' is defined as a boolean
> type.
>
> The initialisation of the formats[] array stores integer 0/1 values into
> this field rather than a more readable bool type.
>
> Convert the usages to boolean true/false, and fix up the local storage
> type in mmal_setup_components() to enhance code readability.
> """

Again I'll agree.

> Reviewed-by: Kieran Bingham <[email protected]>

Reviewed-by: Dave Stevenson <[email protected]>

>
> > Signed-off-by: Umang Jain <[email protected]>
> > ---
> > .../bcm2835-camera/bcm2835-camera.c | 30 +++++++++----------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > index fd456d1f7061..797ebe2a973a 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > @@ -87,21 +87,21 @@ static struct mmal_fmt formats[] = {
> > .depth = 12,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 1,
> > - .remove_padding = 1,
> > + .remove_padding = true,
> > }, {
> > .fourcc = V4L2_PIX_FMT_YUYV,
> > .mmal = MMAL_ENCODING_YUYV,
> > .depth = 16,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 2,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_RGB24,
> > .mmal = MMAL_ENCODING_RGB24,
> > .depth = 24,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 3,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_JPEG,
> > .flags = V4L2_FMT_FLAG_COMPRESSED,
> > @@ -109,7 +109,7 @@ static struct mmal_fmt formats[] = {
> > .depth = 8,
> > .mmal_component = COMP_IMAGE_ENCODE,
> > .ybbp = 0,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_H264,
> > .flags = V4L2_FMT_FLAG_COMPRESSED,
> > @@ -117,7 +117,7 @@ static struct mmal_fmt formats[] = {
> > .depth = 8,
> > .mmal_component = COMP_VIDEO_ENCODE,
> > .ybbp = 0,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_MJPEG,
> > .flags = V4L2_FMT_FLAG_COMPRESSED,
> > @@ -125,63 +125,63 @@ static struct mmal_fmt formats[] = {
> > .depth = 8,
> > .mmal_component = COMP_VIDEO_ENCODE,
> > .ybbp = 0,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_YVYU,
> > .mmal = MMAL_ENCODING_YVYU,
> > .depth = 16,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 2,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_VYUY,
> > .mmal = MMAL_ENCODING_VYUY,
> > .depth = 16,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 2,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_UYVY,
> > .mmal = MMAL_ENCODING_UYVY,
> > .depth = 16,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 2,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_NV12,
> > .mmal = MMAL_ENCODING_NV12,
> > .depth = 12,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 1,
> > - .remove_padding = 1,
> > + .remove_padding = true,
> > }, {
> > .fourcc = V4L2_PIX_FMT_BGR24,
> > .mmal = MMAL_ENCODING_BGR24,
> > .depth = 24,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 3,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > }, {
> > .fourcc = V4L2_PIX_FMT_YVU420,
> > .mmal = MMAL_ENCODING_YV12,
> > .depth = 12,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 1,
> > - .remove_padding = 1,
> > + .remove_padding = true,
> > }, {
> > .fourcc = V4L2_PIX_FMT_NV21,
> > .mmal = MMAL_ENCODING_NV21,
> > .depth = 12,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 1,
> > - .remove_padding = 1,
> > + .remove_padding = true,
> > }, {
> > .fourcc = V4L2_PIX_FMT_BGR32,
> > .mmal = MMAL_ENCODING_BGRA,
> > .depth = 32,
> > .mmal_component = COMP_CAMERA,
> > .ybbp = 4,
> > - .remove_padding = 0,
> > + .remove_padding = false,
> > },
> > };
> >
> > @@ -1147,7 +1147,7 @@ static int mmal_setup_components(struct bcm2835_mmal_dev *dev,
> > struct vchiq_mmal_port *port = NULL, *camera_port = NULL;
> > struct vchiq_mmal_component *encode_component = NULL;
> > struct mmal_fmt *mfmt = get_format(f);
> > - u32 remove_padding;
> > + bool remove_padding;
> >
> > if (!mfmt)
> > return -EINVAL;
> > --
> > 2.38.1
> >