From: Chen-Yu Tsai <wenst@chromium.org> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Philipp Zabel <p.zabel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Chen-Yu Tsai <wenst@chromium.org>, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH RFT v2 8/8] media: hantro: jpeg: Remove open-coded size in quantization table code Date: Fri, 7 Jan 2022 17:34:55 +0800 [thread overview] Message-ID: <20220107093455.73766-9-wenst@chromium.org> (raw) In-Reply-To: <20220107093455.73766-1-wenst@chromium.org> The quantization tables used in the Hantro JPEG encoder driver are implicitly sized by the data they contain, but the loop that scales the tables based on the compression quality hard codes the size to 64. No code exists to check whether the two actually match. Commit 85bdcb7eaae7 ("media: hantro: Write the quantization tables in proper order") introduced two new tables, with sizes hardcoded to 64, but still no checking if all the sizes are the same. Commit 41479adb5e52 ("media: hantro: Avoid global variable for jpeg quantization tables") added the macro JPEG_QUANT_SIZE, but only the newly added fields used this. This has resulted in code scattered with magic numbers and array sizes that happen to match up, without any sort of sanity checking to enforce it. Drop the hard-coded array sizes, replace the magic loop count with a proper JPEG_QUANT_SIZE macro, and add BUILD_BUG_ON()s to check that all the table sizes match up. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/staging/media/hantro/hantro_jpeg.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/staging/media/hantro/hantro_jpeg.c index 84d3f0bfff00..d07b1b449b61 100644 --- a/drivers/staging/media/hantro/hantro_jpeg.c +++ b/drivers/staging/media/hantro/hantro_jpeg.c @@ -49,7 +49,7 @@ static const unsigned char chroma_q_table[] = { 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63 }; -static const unsigned char zigzag[64] = { +static const unsigned char zigzag[] = { 0, 1, 8, 16, 9, 2, 3, 10, 17, 24, 32, 25, 18, 11, 4, 5, 12, 19, 26, 33, 40, 48, 41, 34, @@ -60,7 +60,7 @@ static const unsigned char zigzag[64] = { 53, 60, 61, 54, 47, 55, 62, 63 }; -static const u32 hw_reorder[64] = { +static const u32 hw_reorder[] = { 0, 8, 16, 24, 1, 9, 17, 25, 32, 40, 48, 56, 33, 41, 49, 57, 2, 10, 18, 26, 3, 11, 19, 27, @@ -292,7 +292,10 @@ jpeg_scale_quant_table(unsigned char *file_q_tab, { int i; - for (i = 0; i < 64; i++) { + BUILD_BUG_ON(ARRAY_SIZE(zigzag) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(hw_reorder) != JPEG_QUANT_SIZE); + + for (i = 0; i < JPEG_QUANT_SIZE; i++) { file_q_tab[i] = jpeg_scale_qp(tab[zigzag[i]], scale); reordered_q_tab[i] = jpeg_scale_qp(tab[hw_reorder[i]], scale); } @@ -311,6 +314,11 @@ static void jpeg_set_quality(struct hantro_jpeg_ctx *ctx) else scale = 200 - 2 * ctx->quality; + BUILD_BUG_ON(ARRAY_SIZE(luma_q_table) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(chroma_q_table) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(ctx->hw_luma_qtable) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(ctx->hw_chroma_qtable) != JPEG_QUANT_SIZE); + jpeg_scale_quant_table(ctx->buffer + LUMA_QUANT_OFF, ctx->hw_luma_qtable, luma_q_table, scale); jpeg_scale_quant_table(ctx->buffer + CHROMA_QUANT_OFF, -- 2.34.1.575.g55b058a8bb-goog
WARNING: multiple messages have this Message-ID (diff)
From: Chen-Yu Tsai <wenst@chromium.org> To: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>, Philipp Zabel <p.zabel@pengutronix.de>, Mauro Carvalho Chehab <mchehab@kernel.org>, Hans Verkuil <hverkuil-cisco@xs4all.nl>, Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Chen-Yu Tsai <wenst@chromium.org>, linux-media@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: [PATCH RFT v2 8/8] media: hantro: jpeg: Remove open-coded size in quantization table code Date: Fri, 7 Jan 2022 17:34:55 +0800 [thread overview] Message-ID: <20220107093455.73766-9-wenst@chromium.org> (raw) In-Reply-To: <20220107093455.73766-1-wenst@chromium.org> The quantization tables used in the Hantro JPEG encoder driver are implicitly sized by the data they contain, but the loop that scales the tables based on the compression quality hard codes the size to 64. No code exists to check whether the two actually match. Commit 85bdcb7eaae7 ("media: hantro: Write the quantization tables in proper order") introduced two new tables, with sizes hardcoded to 64, but still no checking if all the sizes are the same. Commit 41479adb5e52 ("media: hantro: Avoid global variable for jpeg quantization tables") added the macro JPEG_QUANT_SIZE, but only the newly added fields used this. This has resulted in code scattered with magic numbers and array sizes that happen to match up, without any sort of sanity checking to enforce it. Drop the hard-coded array sizes, replace the magic loop count with a proper JPEG_QUANT_SIZE macro, and add BUILD_BUG_ON()s to check that all the table sizes match up. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/staging/media/hantro/hantro_jpeg.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/staging/media/hantro/hantro_jpeg.c index 84d3f0bfff00..d07b1b449b61 100644 --- a/drivers/staging/media/hantro/hantro_jpeg.c +++ b/drivers/staging/media/hantro/hantro_jpeg.c @@ -49,7 +49,7 @@ static const unsigned char chroma_q_table[] = { 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63 }; -static const unsigned char zigzag[64] = { +static const unsigned char zigzag[] = { 0, 1, 8, 16, 9, 2, 3, 10, 17, 24, 32, 25, 18, 11, 4, 5, 12, 19, 26, 33, 40, 48, 41, 34, @@ -60,7 +60,7 @@ static const unsigned char zigzag[64] = { 53, 60, 61, 54, 47, 55, 62, 63 }; -static const u32 hw_reorder[64] = { +static const u32 hw_reorder[] = { 0, 8, 16, 24, 1, 9, 17, 25, 32, 40, 48, 56, 33, 41, 49, 57, 2, 10, 18, 26, 3, 11, 19, 27, @@ -292,7 +292,10 @@ jpeg_scale_quant_table(unsigned char *file_q_tab, { int i; - for (i = 0; i < 64; i++) { + BUILD_BUG_ON(ARRAY_SIZE(zigzag) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(hw_reorder) != JPEG_QUANT_SIZE); + + for (i = 0; i < JPEG_QUANT_SIZE; i++) { file_q_tab[i] = jpeg_scale_qp(tab[zigzag[i]], scale); reordered_q_tab[i] = jpeg_scale_qp(tab[hw_reorder[i]], scale); } @@ -311,6 +314,11 @@ static void jpeg_set_quality(struct hantro_jpeg_ctx *ctx) else scale = 200 - 2 * ctx->quality; + BUILD_BUG_ON(ARRAY_SIZE(luma_q_table) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(chroma_q_table) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(ctx->hw_luma_qtable) != JPEG_QUANT_SIZE); + BUILD_BUG_ON(ARRAY_SIZE(ctx->hw_chroma_qtable) != JPEG_QUANT_SIZE); + jpeg_scale_quant_table(ctx->buffer + LUMA_QUANT_OFF, ctx->hw_luma_qtable, luma_q_table, scale); jpeg_scale_quant_table(ctx->buffer + CHROMA_QUANT_OFF, -- 2.34.1.575.g55b058a8bb-goog _______________________________________________ Linux-rockchip mailing list Linux-rockchip@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-rockchip
next prev parent reply other threads:[~2022-01-07 9:35 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-01-07 9:34 [PATCH RFT v2 0/8] media: hantro: jpeg: Various improvements Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 1/8] media: hantro: jpeg: Relax register writes before write starting hardware Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-18 21:02 ` Ezequiel Garcia 2022-01-18 21:02 ` Ezequiel Garcia 2022-01-19 10:08 ` Chen-Yu Tsai 2022-01-19 10:08 ` Chen-Yu Tsai 2022-01-19 11:59 ` Ezequiel Garcia 2022-01-19 11:59 ` Ezequiel Garcia 2022-01-20 14:20 ` Hans Verkuil 2022-01-20 14:20 ` Hans Verkuil 2022-01-21 2:25 ` Chen-Yu Tsai 2022-01-21 2:25 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 2/8] media: hantro: Fix overfill bottom register field name Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 3/8] media: hantro: Support cropping visible area for encoders Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 4/8] media: hantro: jpeg: Add JFIF APP0 segment to JPEG encoder output Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 5/8] media: hantro: jpeg: Add COM segment to JPEG header to align image scan Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 6/8] media: hantro: Implement V4L2_CID_JPEG_ACTIVE_MARKER control Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` [PATCH RFT v2 7/8] media: hantro: output encoded JPEG content directly to capture buffers Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai 2022-01-07 9:34 ` Chen-Yu Tsai [this message] 2022-01-07 9:34 ` [PATCH RFT v2 8/8] media: hantro: jpeg: Remove open-coded size in quantization table code Chen-Yu Tsai
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20220107093455.73766-9-wenst@chromium.org \ --to=wenst@chromium.org \ --cc=ezequiel@vanguardiasur.com.ar \ --cc=gregkh@linuxfoundation.org \ --cc=hverkuil-cisco@xs4all.nl \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=linux-rockchip@lists.infradead.org \ --cc=linux-staging@lists.linux.dev \ --cc=mchehab@kernel.org \ --cc=p.zabel@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.