* [PATCH 0/2] media: coda: Add ability to decode wider range of jpegs @ 2020-03-23 13:09 Adrian Ratiu 2020-03-23 13:09 ` [PATCH 1/2] media: coda: jpeg: support optimized huffman tables Adrian Ratiu 2020-03-23 13:09 ` [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions Adrian Ratiu 0 siblings, 2 replies; 7+ messages in thread From: Adrian Ratiu @ 2020-03-23 13:09 UTC (permalink / raw) To: Philipp Zabel Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey Hello, At Ezequiel's suggestion I'm sending these patches as a standalone series which applies on top of Philipp's v2 decoder series located at Message-Id: <20200318183536.15779-1-p.zabel@pengutronix.de> The use case for these is that we get jpegs from smartphone apps over which we have no control: they run on a variety of OSes / technologies / libraries, could produce jpegs with dimensions from 64x64 to full HD and could use standard or optimized huffman coding. By relaxing these two checks in the decoder we are able to correctly display all our images and think these would also benefit others. Tested with CODA960 on IMX6DL. Kind regards, Adrian Adrian Ratiu (2): media: coda: jpeg: support optimized huffman tables media: coda: be more flexible wrt jpeg dimensions drivers/media/platform/coda/coda-jpeg.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) -- 2.25.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] media: coda: jpeg: support optimized huffman tables 2020-03-23 13:09 [PATCH 0/2] media: coda: Add ability to decode wider range of jpegs Adrian Ratiu @ 2020-03-23 13:09 ` Adrian Ratiu 2020-03-24 9:11 ` Philipp Zabel 2020-03-23 13:09 ` [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions Adrian Ratiu 1 sibling, 1 reply; 7+ messages in thread From: Adrian Ratiu @ 2020-03-23 13:09 UTC (permalink / raw) To: Philipp Zabel Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey Each jpeg can have the huffman tables optimized for its specific content meaning that the table lenghts and values don't match the standard table of substitutions so there's no reason to hardcode and expect the sandard lenghts, otherwise we just end up rejecting optimized jpegs altogether. Tested on CODA960. Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> --- drivers/media/platform/coda/coda-jpeg.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/media/platform/coda/coda-jpeg.c b/drivers/media/platform/coda/coda-jpeg.c index 6a11b64efb6b..162ba28a6b95 100644 --- a/drivers/media/platform/coda/coda-jpeg.c +++ b/drivers/media/platform/coda/coda-jpeg.c @@ -343,7 +343,8 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb) v4l2_err(&dev->v4l2_dev, "missing Huffman table\n"); return -EINVAL; } - if (huffman_tables[i].length != ((i & 2) ? 178 : 28)) { + if (huffman_tables[i].length < 17 || + huffman_tables[i].length > 178) { v4l2_err(&dev->v4l2_dev, "invalid Huffman table %d length: %zu\n", i, huffman_tables[i].length); @@ -357,10 +358,12 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb) return -ENOMEM; ctx->params.jpeg_huff_tab = huff_tab; } - memcpy(huff_tab->luma_dc, huffman_tables[0].start, 16 + 12); - memcpy(huff_tab->chroma_dc, huffman_tables[1].start, 16 + 12); - memcpy(huff_tab->luma_ac, huffman_tables[2].start, 16 + 162); - memcpy(huff_tab->chroma_ac, huffman_tables[3].start, 16 + 162); + + memset(huff_tab, 0, sizeof(*huff_tab)); + memcpy(huff_tab->luma_dc, huffman_tables[0].start, huffman_tables[0].length); + memcpy(huff_tab->chroma_dc, huffman_tables[1].start, huffman_tables[1].length); + memcpy(huff_tab->luma_ac, huffman_tables[2].start, huffman_tables[2].length); + memcpy(huff_tab->chroma_ac, huffman_tables[3].start, huffman_tables[3].length); /* check scan header */ for (i = 0; i < scan_header.num_components; i++) { -- 2.25.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] media: coda: jpeg: support optimized huffman tables 2020-03-23 13:09 ` [PATCH 1/2] media: coda: jpeg: support optimized huffman tables Adrian Ratiu @ 2020-03-24 9:11 ` Philipp Zabel 0 siblings, 0 replies; 7+ messages in thread From: Philipp Zabel @ 2020-03-24 9:11 UTC (permalink / raw) To: Adrian Ratiu Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey Hi Adrian, On Mon, Mar 23, 2020 at 03:09:36PM +0200, Adrian Ratiu wrote: > Each jpeg can have the huffman tables optimized for its specific content > meaning that the table lenghts and values don't match the standard table > of substitutions so there's no reason to hardcode and expect the sandard > lenghts, otherwise we just end up rejecting optimized jpegs altogether. Thank you, that is a great improvement. There's one issue remaining below: > Tested on CODA960. > > Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> > --- > drivers/media/platform/coda/coda-jpeg.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/platform/coda/coda-jpeg.c b/drivers/media/platform/coda/coda-jpeg.c > index 6a11b64efb6b..162ba28a6b95 100644 > --- a/drivers/media/platform/coda/coda-jpeg.c > +++ b/drivers/media/platform/coda/coda-jpeg.c > @@ -343,7 +343,8 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb) > v4l2_err(&dev->v4l2_dev, "missing Huffman table\n"); > return -EINVAL; > } > - if (huffman_tables[i].length != ((i & 2) ? 178 : 28)) { > + if (huffman_tables[i].length < 17 || > + huffman_tables[i].length > 178) { The maximum length of the DC tables is 16 + 12, so this should still be checked for a maximum length of 28 if (i & 2) == 0. > v4l2_err(&dev->v4l2_dev, > "invalid Huffman table %d length: %zu\n", > i, huffman_tables[i].length); > @@ -357,10 +358,12 @@ int coda_jpeg_decode_header(struct coda_ctx *ctx, struct vb2_buffer *vb) > return -ENOMEM; > ctx->params.jpeg_huff_tab = huff_tab; > } > - memcpy(huff_tab->luma_dc, huffman_tables[0].start, 16 + 12); > - memcpy(huff_tab->chroma_dc, huffman_tables[1].start, 16 + 12); > - memcpy(huff_tab->luma_ac, huffman_tables[2].start, 16 + 162); > - memcpy(huff_tab->chroma_ac, huffman_tables[3].start, 16 + 162); > + > + memset(huff_tab, 0, sizeof(*huff_tab)); > + memcpy(huff_tab->luma_dc, huffman_tables[0].start, huffman_tables[0].length); > + memcpy(huff_tab->chroma_dc, huffman_tables[1].start, huffman_tables[1].length); Otherwise these two might overflow the luma_dc and chroma_dc arrays. regards Philipp ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions 2020-03-23 13:09 [PATCH 0/2] media: coda: Add ability to decode wider range of jpegs Adrian Ratiu 2020-03-23 13:09 ` [PATCH 1/2] media: coda: jpeg: support optimized huffman tables Adrian Ratiu @ 2020-03-23 13:09 ` Adrian Ratiu 2020-03-23 14:27 ` Ezequiel Garcia 2020-03-24 9:21 ` Philipp Zabel 1 sibling, 2 replies; 7+ messages in thread From: Adrian Ratiu @ 2020-03-23 13:09 UTC (permalink / raw) To: Philipp Zabel Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey 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 <adrian.ratiu@collabora.com> --- 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions 2020-03-23 13:09 ` [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions Adrian Ratiu @ 2020-03-23 14:27 ` Ezequiel Garcia 2020-03-24 9:21 ` Philipp Zabel 1 sibling, 0 replies; 7+ messages in thread From: Ezequiel Garcia @ 2020-03-23 14:27 UTC (permalink / raw) To: Adrian Ratiu, Philipp Zabel Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, kernel, kernel, Tim Harvey 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 <adrian.ratiu@collabora.com> > --- > 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, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions 2020-03-23 13:09 ` [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions Adrian Ratiu 2020-03-23 14:27 ` Ezequiel Garcia @ 2020-03-24 9:21 ` Philipp Zabel 2020-03-24 11:14 ` Adrian Ratiu 1 sibling, 1 reply; 7+ messages in thread From: Philipp Zabel @ 2020-03-24 9:21 UTC (permalink / raw) To: Adrian Ratiu Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions 2020-03-24 9:21 ` Philipp Zabel @ 2020-03-24 11:14 ` Adrian Ratiu 0 siblings, 0 replies; 7+ messages in thread From: Adrian Ratiu @ 2020-03-24 11:14 UTC (permalink / raw) To: Philipp Zabel Cc: Mauro Carvalho Chehab, linux-media, linux-kernel, Ezequiel Garcia, kernel, kernel, Tim Harvey On Tue, 24 Mar 2020, Philipp Zabel <pza@pengutronix.de> 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-03-24 11:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-03-23 13:09 [PATCH 0/2] media: coda: Add ability to decode wider range of jpegs Adrian Ratiu 2020-03-23 13:09 ` [PATCH 1/2] media: coda: jpeg: support optimized huffman tables Adrian Ratiu 2020-03-24 9:11 ` Philipp Zabel 2020-03-23 13:09 ` [PATCH 2/2] media: coda: be more flexible wrt jpeg dimensions Adrian Ratiu 2020-03-23 14:27 ` Ezequiel Garcia 2020-03-24 9:21 ` Philipp Zabel 2020-03-24 11:14 ` Adrian Ratiu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).