| Message ID | 20180129163435.24936-19-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Niklas, Thank you for the patch. On Monday, 29 January 2018 18:34:23 EET Niklas Söderlund wrote: > Add a check to ensure the colorspace from user-space is good. On Gen2 it > works without this change as the sensor sets the colorspace but on Gen3 > this can fail if the colorspace provided by the user is not good. The > values to check for comes from v4l2-compliance sources which is the tool > that found this error. If this check is not preformed v4l2-compliance s/preformed/performed/ > fails when it tests colorspace. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/rcar-vin/rcar-v4l2.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c > b/drivers/media/platform/rcar-vin/rcar-v4l2.c index > 841d62ca27e026d7..6403650aff22a2ed 100644 > --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c > +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c > @@ -23,6 +23,7 @@ > #include "rcar-vin.h" > > #define RVIN_DEFAULT_FORMAT V4L2_PIX_FMT_YUYV > +#define RVIN_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB > > /* ------------------------------------------------------------------------ > * Format Conversions > @@ -115,6 +116,10 @@ static int rvin_format_align(struct rvin_dev *vin, > struct v4l2_pix_format *pix) break; > } > > + /* Check that colorspace is reasonable */ > + if (!pix->colorspace || pix->colorspace >= 0xff) I'd write the first check as pix->colorspace == V4L2_COLORSPACE_DEFAULT For the second check I don't think 0xff is a meaningful value. We currently have 12 colorspaces defined. If we want to be future-proof I'd add a V4L2_COLORSPACE_MAX entry to enum v4l2_colorspace and use that for the check. Alternatively you could use V4L2_COLORSPACE_DCI_P3 but the driver would need to be updated when new colorspaces get added. > + pix->colorspace = RVIN_DEFAULT_COLORSPACE; > + > /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */ > walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c index 841d62ca27e026d7..6403650aff22a2ed 100644 --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c @@ -23,6 +23,7 @@ #include "rcar-vin.h" #define RVIN_DEFAULT_FORMAT V4L2_PIX_FMT_YUYV +#define RVIN_DEFAULT_COLORSPACE V4L2_COLORSPACE_SRGB /* ----------------------------------------------------------------------------- * Format Conversions @@ -115,6 +116,10 @@ static int rvin_format_align(struct rvin_dev *vin, struct v4l2_pix_format *pix) break; } + /* Check that colorspace is reasonable */ + if (!pix->colorspace || pix->colorspace >= 0xff) + pix->colorspace = RVIN_DEFAULT_COLORSPACE; + /* HW limit width to a multiple of 32 (2^5) for NV16 else 2 (2^1) */ walign = vin->format.pixelformat == V4L2_PIX_FMT_NV16 ? 5 : 1;
Add a check to ensure the colorspace from user-space is good. On Gen2 it works without this change as the sensor sets the colorspace but on Gen3 this can fail if the colorspace provided by the user is not good. The values to check for comes from v4l2-compliance sources which is the tool that found this error. If this check is not preformed v4l2-compliance fails when it tests colorspace. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-v4l2.c | 5 +++++ 1 file changed, 5 insertions(+)