[PATCH 6/7] media: verisilicon: Enforce a minimum sizeimage for the JPEG decoder

From: Lucas Sinn <[email protected]>

The VPU720 JPEG decoder is driven like a stateful decoder: userspace
hands it a whole frame per source buffer and never looks inside the
bitstream, so it has no way of knowing how large a frame can get.
GStreamer picks a source buffer size of its own, which is large enough at
low resolutions and too small once the resolution grows.

A frame that does not fit is lost.  There is no way to report a partially
consumed buffer, so the driver can only refuse it;
rockchip_vpu720_jpeg_dec_run() notices by finding no EOI marker at the end
of the payload and returns -EINVAL.  Every frame then fails, which is a
poor way to tell an application that its buffers are too small.

For coded formats hantro_try_fmt() computes the worst case frame size and
uses it only when the application asks for zero.  Use it as a lower bound
for the JPEG decoder instead, so an application that asks for less gets
buffers that can hold any frame of the negotiated resolution.  The other
codecs are stateless, userspace parses the bitstream itself and knows the
frame sizes, so they keep the existing behaviour.

This is not free.  max_depth is 2 for V4L2_PIX_FMT_JPEG, so a 1080p source
buffer cannot be smaller than 4 MiB and a 4K one not smaller than 16 MiB,
while a typical JPEG frame is an order of magnitude below that.  The
alternative is to let userspace keep its own size and take the decode
failures, which is worse in practice.

Signed-off-by: Lucas Sinn <[email protected]>
Signed-off-by: Sascha Hauer <[email protected]>
---
 drivers/media/platform/verisilicon/hantro_v4l2.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/verisilicon/hantro_v4l2.c b/drivers/media/platform/verisilicon/hantro_v4l2.c
index b9b1848e43b30..156d0c5453e5a 100644
--- a/drivers/media/platform/verisilicon/hantro_v4l2.c
+++ b/drivers/media/platform/verisilicon/hantro_v4l2.c
@@ -386,14 +386,26 @@ static int hantro_try_fmt(const struct hantro_ctx *ctx,
 			pix_mp->plane_fmt[0].sizeimage +=
 				hantro_av1_mv_size(pix_mp->width,
 						   pix_mp->height);
-	} else if (!pix_mp->plane_fmt[0].sizeimage) {
+	} else {
+		u32 sizeimage = fmt->header_size +
+				pix_mp->width * pix_mp->height * fmt->max_depth;
+
 		/*
 		 * For coded formats the application can specify
 		 * sizeimage. If the application passes a zero sizeimage,
 		 * let's default to the maximum frame size.
+		 *
+		 * The JPEG decoder is the exception. Applications drive it
+		 * without parsing the bitstream, so they cannot know how large
+		 * a frame gets and pick a size that works at low resolutions
+		 * and silently truncates frames further up. Treat the maximum
+		 * as a minimum there.
 		 */
-		pix_mp->plane_fmt[0].sizeimage = fmt->header_size +
-			pix_mp->width * pix_mp->height * fmt->max_depth;
+		if (fmt->codec_mode == HANTRO_MODE_JPEG_DEC)
+			pix_mp->plane_fmt[0].sizeimage =
+				max(pix_mp->plane_fmt[0].sizeimage, sizeimage);
+		else if (!pix_mp->plane_fmt[0].sizeimage)
+			pix_mp->plane_fmt[0].sizeimage = sizeimage;
 	}
 
 	return 0;

-- 
2.47.3


_______________________________________________
Linux-rockchip mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-rockchip