linux-kernel - Re: [PATCH 4/5] media: imx-jpeg: Add V4L2 driver for i.MX8 JPEG Encoder/Decoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <58ed179388b6f9d6caddd91dc03e411ca714157d.camel@collabora.com>
Date:   Wed, 13 Nov 2019 11:00:35 -0300
From:   Ezequiel Garcia <ezequiel@...labora.com>
To:     Mirela Rabulea <mirela.rabulea@....com>, mchehab@...nel.org,
        shawnguo@...nel.org, robh+dt@...nel.org
Cc:     hverkuil-cisco@...all.nl, paul.kocialkowski@...tlin.com,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-imx@....com, s.hauer@...gutronix.de, aisheng.dong@....com,
        daniel.baluta@....com, leonard.crestez@....com,
        robert.chiras@....com, laurentiu.palcu@....com,
        mark.rutland@....com, devicetree@...r.kernel.org,
        p.zabel@...gutronix.de, laurent.pinchart+renesas@...asonboard.com,
        niklas.soderlund+renesas@...natech.se,
        dafna.hirschfeld@...labora.com
Subject: Re: [PATCH 4/5] media: imx-jpeg: Add V4L2 driver for i.MX8 JPEG
 Encoder/Decoder

Hi Mirela,

Thanks for the patch. It's nice to see support for i.MX8!

On Wed, 2019-11-06 at 17:20 +0200, Mirela Rabulea wrote:
> V4L2 driver for the JPEG encoder/decoder from i.MX8QXP/i.MX8QM application
> processors.
> The multi-planar buffers API is used.
> 
> Baseline and extended sequential jpeg decoding is supported.
> Progressive jpeg decoding is not supported by the IP.
> Supports encode and decode of various formats:
>      YUV444, YUV422, YUV420, RGB, ARGB, Gray
> YUV420 is the only multi-planar format supported.
> Minimum resolution is 64 x 64, maximum 8192 x 8192.
> The alignment requirements for the resolution depend on the format,
> multiple of 16 resolutions should work for all formats.
> 
> Signed-off-by: Mirela Rabulea <mirela.rabulea@....com>
> ---
>  drivers/media/platform/Kconfig                |    2 +
>  drivers/media/platform/Makefile               |    1 +
>  drivers/media/platform/imx-jpeg/Kconfig       |   10 +
>  drivers/media/platform/imx-jpeg/Makefile      |    3 +
>  drivers/media/platform/imx-jpeg/mxc-jpeg-hw.c |  168 ++
>  drivers/media/platform/imx-jpeg/mxc-jpeg-hw.h |  140 ++
>  drivers/media/platform/imx-jpeg/mxc-jpeg.c    | 2266 +++++++++++++++++++++++++
>  drivers/media/platform/imx-jpeg/mxc-jpeg.h    |  187 ++
>  8 files changed, 2777 insertions(+)
>  create mode 100644 drivers/media/platform/imx-jpeg/Kconfig
>  create mode 100644 drivers/media/platform/imx-jpeg/Makefile
>  create mode 100644 drivers/media/platform/imx-jpeg/mxc-jpeg-hw.c
>  create mode 100644 drivers/media/platform/imx-jpeg/mxc-jpeg-hw.h
>  create mode 100644 drivers/media/platform/imx-jpeg/mxc-jpeg.c
>  create mode 100644 drivers/media/platform/imx-jpeg/mxc-jpeg.h
> 
> [..]

> +};
> +
> +/*
> + * default configuration stream, 64x64 yuv422
> + * split by JPEG marker, so it's easier to modify & use
> + */
> +static const unsigned char jpeg_soi[] = {0xFF, 0xD8};
> +static const unsigned char jpeg_app0[] = {0xFF, 0xE0,

I think it's time to re-consider creating some common code
for drivers that deal with JPEG parsing. I don't know
exactly how this should be done, but it's worth a try.

Having been there, it might sound unfair to request you to
take such action. However, someone has to do these kinds
of cleanups and improvements, sooner or later, if we want
to keep a clean codebase.

Hope this makes sense.

Regards,
Ezequiel