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. Regards, Hans
