Hi Hans,
W dniu 17.11.2021 o 11:51, Andrzej Pietrasiewicz pisze:
Hi again,
W dniu 17.11.2021 o 11:49, Andrzej Pietrasiewicz pisze:
Hi,
W dniu 17.11.2021 o 10:59, Hans Verkuil pisze:
On 16/11/2021 14:14, Andrzej Pietrasiewicz wrote:
Hi,
W dniu 16.11.2021 o 09:21, Hans Verkuil pisze:
On 16/11/2021 09:09, Andrzej Pietrasiewicz wrote:
Hi Hans,
W dniu 15.11.2021 o 22:16, Hans Verkuil pisze:
On 15/11/2021 18:14, Andrzej Pietrasiewicz wrote:
Hi Hans,
W dniu 15.11.2021 o 16:07, Hans Verkuil pisze:
Andrzej,
Can you rebase this series on top of the master branch of
https://git.linuxtv.org/media_stage.git/ ? Unfortunately this v7 no longer
applies. Specifically "rkvdec: Add the VP9 backend" failed in a
non-trivial
manner.
This is a branch for you:
https://gitlab.collabora.com/linux/for-upstream/-/tree/vp9-uapi
I'm getting a bunch of sparse/smatch warnings:
Thanks for finding this, I will re-create the branch and let you know on irc.
Some of the below are "false positives, namely:
drivers/media/platform/omap3isp/omap3isp.h
drivers/media/platform/qcom/venus/core.h
Ah, sorry, I though I had filtered those out. Obviously you can ignore those.
Please post a v8. That way the series is archived on lore. And it works better
with patchwork.
Sure, no problem. Also please see below.
Regards,
Hans
which are not touched by the series.
Regards,
Andrzej
sparse:
rkvdec/rkvdec-vp9.c:190:43: warning: variable 'dec_params' set but not
used [-Wunused-but-set-variable]
rkvdec/rkvdec-vp9.c:245:43: warning: variable 'dec_params' set but not
used [-Wunused-but-set-variable]
SPARSE:hantro/hantro_postproc.c hantro/hantro_postproc.c:37:35: warning:
symbol 'hantro_g1_postproc_regs' was not declared. Should it be static?
smatch:
rkvdec/rkvdec-vp9.c:190:43: warning: variable 'dec_params' set but not
used [-Wunused-but-set-variable]
rkvdec/rkvdec-vp9.c:245:43: warning: variable 'dec_params' set but not
used [-Wunused-but-set-variable]
rkvdec/rkvdec-vp9.c: rkvdec/rkvdec-vp9.c:236 init_intra_only_probs()
error: buffer overflow 'ptr' 90 <= 91
this looks a false positive.
A portion of memory pointed to by ptr is indexed with i * 23 + m,
where i ranges from 0 to 3, inclusive, and m ranges from 0 to 22,
inclusive if i < 3, otherwise m ranges from 0 to 20, inclusive.
So the largest index value we compute equals 89 (3 * 23 + 20).
Because ptr points to something that is at least 90 bytes large,
89 is a valid index and no greater index will be ever computed.
But we do need to get rid of this smatch warning, otherwise it will pollute the
list of smatch warnings.
I was looking at the code and wonder if it wouldn't make more sense to
move writing to rkprobs->intra_mode[i].uv_mode[] into a separate for loop:
for (i = 0; i < ARRAY_SIZE(v4l2_vp9_kf_uv_mode_prob); i++)
rkprobs->intra_mode[i / 23].uv_mode[i % 23] =
v4l2_vp9_kf_uv_mode_prob[i];
Wouldn't that do the same as the current code? It looks simpler as well.
I think it would, but I would slightly change the loop:
for (i = 0; i < ARRAY_SIZE(v4l2_vp9_kf_uv_mode_prob); i++) {
actually, sizeof(v4l2_vp9_kf_uv_mode_prob)
const u8 *ptr = (const u8 *)v4l2_vp9_kf_uv_mode_prob;
rkprobs->intra_mode[i / 23].uv_mode[i % 23] = ptr[i];
}
because v4l2_vp9_kf_uv_mode_prob is actually a u8[10][9].
I will make such a change locally and test whether it causes regressions.
This worked, no regressions:
for (i = 0; i < sizeof(v4l2_vp9_kf_uv_mode_prob); ++i) {
const u8 *ptr = (const u8 *)v4l2_vp9_kf_uv_mode_prob;
rkprobs->intra_mode[i / 23].uv_mode[i % 23] = ptr[i];
}
Andrzej
