media: rpivid: Avoid returning EINVAL to a G_FMT ioctl

V4L2 spec says that G/S/TRY_FMT IOCTLs should never return errors for
anything other than wrong buffer types. Improve the capture format
function such that this is so and unsupported values get converted
to supported ones properly.

Signed-off-by: John Cox <[email protected]>

Author: jc-kynesim

drivers/staging/media/rpivid/rpivid.c

@@ -249,7 +249,6 @@ static int rpivid_open(struct file *file)
 	/* The only bit of format info that we can guess now is H265 src
 	 * Everything else we need more info for
 	 */
-	ctx->src_fmt.pixelformat = RPIVID_SRC_PIXELFORMAT_DEFAULT;
 	rpivid_prepare_src_format(&ctx->src_fmt);
 
 	v4l2_fh_add(&ctx->fh);

drivers/staging/media/rpivid/rpivid.h

@@ -35,8 +35,6 @@
 
 #define RPIVID_QUIRK_NO_DMA_OFFSET	BIT(0)
 
-#define RPIVID_SRC_PIXELFORMAT_DEFAULT	V4L2_PIX_FMT_HEVC_SLICE
-
 enum rpivid_irq_status {
 	RPIVID_IRQ_NONE,
 	RPIVID_IRQ_ERROR,

drivers/staging/media/rpivid/rpivid_video.c

@@ -27,6 +27,8 @@
 
 #define RPIVID_MIN_WIDTH	16U
 #define RPIVID_MIN_HEIGHT	16U
+#define RPIVID_DEFAULT_WIDTH	1920U
+#define RPIVID_DEFAULT_HEIGHT	1088U
 #define RPIVID_MAX_WIDTH	4096U
 #define RPIVID_MAX_HEIGHT	4096U
 
@@ -70,25 +72,22 @@ size_t rpivid_bit_buf_size(unsigned int w, unsigned int h, unsigned int bits_min
 	return rpivid_round_up_size(bits_alloc);
 }
 
-int rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt)
+void rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt)
 {
 	size_t size;
 	u32 w;
 	u32 h;
 
-	if (pix_fmt->pixelformat != V4L2_PIX_FMT_HEVC_SLICE)
-		return -EINVAL;
-
 	w = pix_fmt->width;
 	h = pix_fmt->height;
 	if (!w || !h) {
-		w = 1920;
-		h = 1080;
+		w = RPIVID_DEFAULT_WIDTH;
+		h = RPIVID_DEFAULT_HEIGHT;
 	}
-	if (w > 4096)
-		w = 4096;
-	if (h > 4096)
-		h = 4096;
+	if (w > RPIVID_MAX_WIDTH)
+		w = RPIVID_MAX_WIDTH;
+	if (h > RPIVID_MAX_HEIGHT)
+		h = RPIVID_MAX_HEIGHT;
 
 	if (!pix_fmt->plane_fmt[0].sizeimage ||
 	    pix_fmt->plane_fmt[0].sizeimage > SZ_32M) {
@@ -98,29 +97,41 @@ int rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt)
 	/* Set a minimum */
 	size = max_t(u32, SZ_4K, pix_fmt->plane_fmt[0].sizeimage);
 
+	pix_fmt->pixelformat = V4L2_PIX_FMT_HEVC_SLICE;
 	pix_fmt->width = w;
 	pix_fmt->height = h;
 	pix_fmt->num_planes = 1;
 	pix_fmt->field = V4L2_FIELD_NONE;
 	/* Zero bytes per line for encoded source. */
 	pix_fmt->plane_fmt[0].bytesperline = 0;
 	pix_fmt->plane_fmt[0].sizeimage = size;
-
-	return 0;
 }
 
-int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
+/* Take any pix_format and make it valid */
+static void rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
 {
 	unsigned int width = pix_fmt->width;
 	unsigned int height = pix_fmt->height;
 	unsigned int sizeimage = pix_fmt->plane_fmt[0].sizeimage;
 	unsigned int bytesperline = pix_fmt->plane_fmt[0].bytesperline;
 
-	switch (pix_fmt->pixelformat) {
+	if (!width)
+		width = RPIVID_DEFAULT_WIDTH;
+	if (width > RPIVID_MAX_WIDTH)
+		width = RPIVID_MAX_WIDTH;
+	if (!height)
+		height = RPIVID_DEFAULT_HEIGHT;
+	if (height > RPIVID_MAX_HEIGHT)
+		height = RPIVID_MAX_HEIGHT;
+
 	/* For column formats set bytesperline to column height (stride2) */
+	switch (pix_fmt->pixelformat) {
+	default:
+		pix_fmt->pixelformat = V4L2_PIX_FMT_NV12_COL128;
+		fallthrough;
 	case V4L2_PIX_FMT_NV12_COL128:
 		/* Width rounds up to columns */
-		width = ALIGN(min(width, RPIVID_MAX_WIDTH), 128);
+		width = ALIGN(width, 128);
 
 		/* 16 aligned height - not sure we even need that */
 		height = ALIGN(height, 16);
@@ -140,7 +151,7 @@ int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
 		/* width in pixels (3 pels = 4 bytes) rounded to 128 byte
 		 * columns
 		 */
-		width = ALIGN(((min(width, RPIVID_MAX_WIDTH) + 2) / 3), 32) * 3;
+		width = ALIGN(((width + 2) / 3), 32) * 3;
 
 		/* 16-aligned height. */
 		height = ALIGN(height, 16);
@@ -157,9 +168,6 @@ int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
 		sizeimage = constrain2x(sizeimage,
 					bytesperline * width * 4 / 3);
 		break;
-
-	default:
-		return -EINVAL;
 	}
 
 	pix_fmt->width = width;
@@ -169,7 +177,6 @@ int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt)
 	pix_fmt->plane_fmt[0].bytesperline = bytesperline;
 	pix_fmt->plane_fmt[0].sizeimage = sizeimage;
 	pix_fmt->num_planes = 1;
-	return 0;
 }
 
 static int rpivid_querycap(struct file *file, void *priv,
@@ -260,14 +267,13 @@ static u32 pixelformat_from_sps(const struct v4l2_ctrl_hevc_sps * const sps,
 {
 	u32 pf = 0;
 
-	// Use width 0 as a signifier of unsetness
-	if (!is_sps_set(sps)) {
+	if (!is_sps_set(sps) || !rpivid_hevc_validate_sps(sps)) {
 		/* Treat this as an error? For now return both */
 		if (index == 0)
 			pf = V4L2_PIX_FMT_NV12_COL128;
 		else if (index == 1)
 			pf = V4L2_PIX_FMT_NV12_10_COL128;
-	} else if (index == 0 && rpivid_hevc_validate_sps(sps)) {
+	} else if (index == 0) {
 		if (sps->bit_depth_luma_minus8 == 0)
 			pf = V4L2_PIX_FMT_NV12_COL128;
 		else if (sps->bit_depth_luma_minus8 == 2)
@@ -282,11 +288,14 @@ rpivid_hevc_default_dst_fmt(struct rpivid_ctx * const ctx)
 {
 	const struct v4l2_ctrl_hevc_sps * const sps =
 		rpivid_find_control_data(ctx, V4L2_CID_MPEG_VIDEO_HEVC_SPS);
-	struct v4l2_pix_format_mplane pix_fmt = {
-		.width = sps->pic_width_in_luma_samples,
-		.height = sps->pic_height_in_luma_samples,
-		.pixelformat = pixelformat_from_sps(sps, 0)
-	};
+	struct v4l2_pix_format_mplane pix_fmt;
+
+	memset(&pix_fmt, 0, sizeof(pix_fmt));
+	if (is_sps_set(sps)) {
+		pix_fmt.width = sps->pic_width_in_luma_samples;
+		pix_fmt.height = sps->pic_height_in_luma_samples;
+		pix_fmt.pixelformat = pixelformat_from_sps(sps, 0);
+	}
 
 	rpivid_prepare_dst_format(&pix_fmt);
 	return pix_fmt;
@@ -315,14 +324,23 @@ static int rpivid_enum_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
+/*
+ * get dst format - sets it to default if otherwise unset
+ * returns a pointer to the struct as a convienience
+ */
+static struct v4l2_pix_format_mplane *get_dst_fmt(struct rpivid_ctx *const ctx)
+{
+	if (!ctx->dst_fmt_set)
+		ctx->dst_fmt = rpivid_hevc_default_dst_fmt(ctx);
+	return &ctx->dst_fmt;
+}
+
 static int rpivid_g_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct rpivid_ctx *ctx = rpivid_file2ctx(file);
 
-	if (!ctx->dst_fmt_set)
-		ctx->dst_fmt = rpivid_hevc_default_dst_fmt(ctx);
-	f->fmt.pix_mp = ctx->dst_fmt;
+	f->fmt.pix_mp = *get_dst_fmt(ctx);
 	return 0;
 }
 
@@ -358,31 +376,20 @@ static int rpivid_try_fmt_vid_cap(struct file *file, void *priv,
 			break;
 	}
 
-	// If we can't use requested fmt then set to default
-	if (pixelformat == 0) {
-		pixelformat = pixelformat_from_sps(sps, 0);
-		// If we don't have a default then give up
-		if (pixelformat == 0)
-			return -EINVAL;
-	}
-
 	// We don't have any way of finding out colourspace so believe
 	// anything we are told - take anything set in src as a default
 	if (f->fmt.pix_mp.colorspace == V4L2_COLORSPACE_DEFAULT)
 		copy_color(&f->fmt.pix_mp, &ctx->src_fmt);
 
 	f->fmt.pix_mp.pixelformat = pixelformat;
-	return rpivid_prepare_dst_format(&f->fmt.pix_mp);
+	rpivid_prepare_dst_format(&f->fmt.pix_mp);
+	return 0;
 }
 
 static int rpivid_try_fmt_vid_out(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	if (rpivid_prepare_src_format(&f->fmt.pix_mp)) {
-		// Set default src format
-		f->fmt.pix_mp.pixelformat = RPIVID_SRC_PIXELFORMAT_DEFAULT;
-		rpivid_prepare_src_format(&f->fmt.pix_mp);
-	}
+	rpivid_prepare_src_format(&f->fmt.pix_mp);
 	return 0;
 }
 
@@ -474,7 +481,7 @@ static int rpivid_queue_setup(struct vb2_queue *vq, unsigned int *nbufs,
 	if (V4L2_TYPE_IS_OUTPUT(vq->type))
 		pix_fmt = &ctx->src_fmt;
 	else
-		pix_fmt = &ctx->dst_fmt;
+		pix_fmt = get_dst_fmt(ctx);
 
 	if (*nplanes) {
 		if (sizes[0] < pix_fmt->plane_fmt[0].sizeimage)

drivers/staging/media/rpivid/rpivid_video.h

@@ -28,7 +28,6 @@ int rpivid_queue_init(void *priv, struct vb2_queue *src_vq,
 size_t rpivid_bit_buf_size(unsigned int w, unsigned int h, unsigned int bits_minus8);
 size_t rpivid_round_up_size(const size_t x);
 
-int rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt);
-int rpivid_prepare_dst_format(struct v4l2_pix_format_mplane *pix_fmt);
+void rpivid_prepare_src_format(struct v4l2_pix_format_mplane *pix_fmt);
 
 #endif