Re: [PATCH v3 00/14] staging: media: zoran: fusion in one module

[Hans Verkuil <hverkuil@xs4all.nl>]

On 07/11/2021 17:35, LABBE Corentin wrote:
> Le Wed, Nov 03, 2021 at 05:29:46PM +0100, Hans Verkuil a écrit :
>> On 03/11/2021 16:57, LABBE Corentin wrote:
>>> Le Wed, Nov 03, 2021 at 04:21:02PM +0100, Hans Verkuil a écrit :
>>>> Hi Corentin,
>>>>
>>>> On 26/10/2021 21:34, Corentin Labbe wrote:
>>>>> Hello
>>>>>
>>>>> The main change of this serie is to fusion all zoran related modules in
>>>>> one.
>>>>> This fixes the load order problem when everything is built-in.
>>>>
>>>> I've been testing this series, and while the module load/unload is now working,
>>>> I'm running into a lot of other v4l2 compliance issues.
>>>>
>>>> I've fixed various issues in some follow-up patches available in my tree:
>>>>
>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran
>>>>
>>>> At least some of the worst offenders are now resolved. Note that the patch
>>>> dropping read/write support relies on this patch:
>>>>
>>>> https://patchwork.linuxtv.org/project/linux-media/patch/4f89b139-13b7-eee6-9662-996626b778b0@xs4all.nl/
>>>
>>> Hello
>>>
>>> My test branch already included your "zoran: fix various V4L2 compliance errors"
>>> I have quickly checked other patch and I am ok with them.
>>> I will add and test with them.
>>>
>>>>
>>>> But there is one really major bug that makes me hesitant to merge this:
>>>>
>>>> This works:
>>>>
>>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=576
>>>> v4l2-ctl --stream-mmap
>>>>
>>>> This fails:
>>>>
>>>> v4l2-ctl -v pixelformat=MJPG,width=768,height=288
>>>> v4l2-ctl --stream-mmap
>>>>
>>>> It's an immediate lock up with nothing to indicate what is wrong.
>>>> As soon as the height is 288 or less, this happens.
>>>>
>>>> Both with my DC30 and DC30D.
>>>
>>> Just for curiosity, what is the difference between thoses two ?
>>
>> It's the DC30 variant without an adv7175.
>>
>>>
>>>>
>>>> Do you see the same? Any idea what is going on? I would feel much happier
>>>> if this is fixed.
>>>>
>>>> Note that the same problem is present without this patch series, so it's
>>>> been there for some time.
>>>>
>>>
>>> I will start on digging this problem and add thoses commands to my CI.
>>> And I know there are a huge quantity of problem since origins.
>>> A simple example is that just setting MJPEG as default input format does not work.
>>>
>>> But since it is not related to my serie, can you please merge it.
>>
>> Before I do that, I would really like to know a bit more about this issue:
>> can you reproduce it? Is it DC30 specific or a general problem with zoran?
>>
>> The problem with this hard hang is that it is hard to do regression testing
>> with v4l2-compliance, since it will hang as soon as MJPG pixelformat is
>> tested.
>>
>> I would feel much happier if the hang can be avoided, even if it is just
>> with a temporary hack. It will make it much easier going forward.
>>
> 
> I found the bug
> 
> The null pointer deref was in zoran_reap_stat_com() due to 
> buf = zr->inuse[i];
> ...
> buf->vbuf.vb2_buf.timestamp = ktime_get_ns();
> with buf = NULL;
> 
> It is due to miscalculation of "i".
> 
> I will resend my serie with the fix for that.

Excellent news! Thank you for tracking this one down.

When you post your series, can you include my patches from
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=zoran as well?

Regards,

	Hans