2020-03-23 13:10:03

by Adrian Ratiu

[permalink] [raw]
Subject: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions

Don't require jpeg dimensions to exactly match format dimensions,
so we are able to decode and display a wider range jpegs instead
of outright rejecting the ones which don't match.

This is useful in applications which pass jpegs with arbitrary
dimensions, where buffers can be reused to decode smaller jpegs
without having to do expensive renegotiations.

Signed-off-by: Adrian Ratiu <[email protected]>
---
drivers/media/platform/coda/coda-jpeg.c | 7 -------
1 file changed, 7 deletions(-)

diff --git a/drivers/media/platform/coda/coda-jpeg.c b/drivers/media/platform/coda/coda-jpeg.c
index 162ba28a6b95..782a78dcaf4d 100644
--- a/drivers/media/platform/coda/coda-jpeg.c
+++ b/drivers/media/platform/coda/coda-jpeg.c
@@ -302,13 +302,6 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb)
}

q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
- if (header.frame.height != q_data_src->height ||
- header.frame.width != q_data_src->width) {
- v4l2_err(&dev->v4l2_dev,
- "dimensions don't match format: %dx%d\n",
- header.frame.width, header.frame.height);
- return -EINVAL;
- }

if (header.frame.num_components != 3) {
v4l2_err(&dev->v4l2_dev,
--
2.25.2


2020-03-23 14:28:25

by Ezequiel Garcia

[permalink] [raw]
Subject: Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions

On Mon, 2020-03-23 at 15:09 +0200, Adrian Ratiu wrote:
> Don't require jpeg dimensions to exactly match format dimensions,
> so we are able to decode and display a wider range jpegs instead
> of outright rejecting the ones which don't match.
>
> This is useful in applications which pass jpegs with arbitrary
> dimensions, where buffers can be reused to decode smaller jpegs
> without having to do expensive renegotiations.
>
> Signed-off-by: Adrian Ratiu <[email protected]>
> ---
> drivers/media/platform/coda/coda-jpeg.c | 7 -------
> 1 file changed, 7 deletions(-)
>
> diff --git a/drivers/media/platform/coda/coda-jpeg.c b/drivers/media/platform/coda/coda-jpeg.c
> index 162ba28a6b95..782a78dcaf4d 100644
> --- a/drivers/media/platform/coda/coda-jpeg.c
> +++ b/drivers/media/platform/coda/coda-jpeg.c
> @@ -302,13 +302,6 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb)
> }
>
> q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> - if (header.frame.height != q_data_src->height ||
> - header.frame.width != q_data_src->width) {
> - v4l2_err(&dev->v4l2_dev,
> - "dimensions don't match format: %dx%d\n",
> - header.frame.width, header.frame.height);
> - return -EINVAL;
> - }

Since you are dropping this check, and allowing other sizes to be
decoded using, do you have any check to make sure you don't overrun
(in bytes, not in width x height) the CAPTURE (decoded) buffer?

Also, IIUC your application is negotiating W x H, but then
passing a different size: I wonder if this is not an abuse
of the spec?

Thanks,
Ezequiel

>
> if (header.frame.num_components != 3) {
> v4l2_err(&dev->v4l2_dev,


2020-03-24 09:23:02

by Philipp Zabel

[permalink] [raw]
Subject: Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions

Hi Adrian,

On Mon, Mar 23, 2020 at 03:09:37PM +0200, Adrian Ratiu wrote:
> Don't require jpeg dimensions to exactly match format dimensions,
> so we are able to decode and display a wider range jpegs instead
> of outright rejecting the ones which don't match.

I don't think this is right. If userspace feeds us an incomatible
JPEG we should probably stop decoding and send a source change
event instead [1].

[1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/dev-decoder.html#dynamic-resolution-change

regards
Philipp

2020-03-24 11:14:45

by Adrian Ratiu

[permalink] [raw]
Subject: Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions

On Tue, 24 Mar 2020, Philipp Zabel <[email protected]> wrote:
> Hi Adrian,
>
> On Mon, Mar 23, 2020 at 03:09:37PM +0200, Adrian Ratiu wrote:
>> Don't require jpeg dimensions to exactly match format
>> dimensions, so we are able to decode and display a wider range
>> jpegs instead of outright rejecting the ones which don't match.
>
> I don't think this is right. If userspace feeds us an
> incomatible JPEG we should probably stop decoding and send a
> source change event instead [1].

Please ignore this second patch as it abuses the spec as Ezequiel
mentioned in the other reply. The correct approach would be to
do a format renegotiation in userspace so the test can be passed.

I will send an updated v2 of the huffman table patch based on
your feedback.

Thank you,
Adrian

>
> [1] https://linuxtv.org/downloads/v4l-dvb-apis-new/uapi/v4l/dev-decoder.html#dynamic-resolution-change
>
> regards
> Philipp