Re: [PATCH v2 14/14] [media] s5p-mfc: Don't change the image size to smaller than the request.

[Hans Verkuil <hverkuil@xs4all.nl>]

Hi,

Let me chime in as well, based on the discussions I had last week in
Düsseldorf:

On 10/09/2014 12:06 PM, Kamil Debski wrote:
> Hi,
>
>> From: Nicolas Dufresne [mailto:nicolas.dufresne@collabora.com]
>> Sent: Wednesday, October 08, 2014 4:35 PM
>>
>>
>> Le 2014-10-08 06:24, Kamil Debski a écrit :
>>> Hi,
>>>
>>> This patch seems complicated and I do not understand your motives.
>>>
>>> Could you explain what is the problem with the current aligning of
>> the
>>> values?
>>> Is this a hardware problem? Which MFC version does it affect?
>>> Is it a software problem? If so, maybe the user space application
>> should
>>> take extra care on what value it passes/receives to try_fmt?
>> This looks like something I wanted to bring here as an RFC but never
>> manage to get the time. In an Odroid Integration we have started using
>> the following simple patch to work around this:
>>
>> https://github.com/dsd/linux-
>> odroid/commit/c76b38c1d682b9870ea3b00093ad6500a9c5f5f6
>>
>> The context is that right now we have decided that alignment in s_fmt
>> shall be done with a closest rounding. So the format returned may be
>> bigger, or smaller, that's basically random. I've been digging through
>> a
>> lot, and so far I have found no rational that explains this choice
>> other
>> that this felt right.
>>
>> In real life, whenever the resulting format is smaller then request,
>> there is little we can do other then fail or try again blindly other
>> sizes. But with bigger raw buffers, we can use zero-copy  cropping
>> techniques to keep going. Here's a example:
>>
>> image_generator -> hw_converter -> display
>>
>> As hw_converter is a V4L2 M2M, an ideal use case here would be for
>> image_generator to use buffers from the hw_converter. For the scenario,
>> it is likely that a fixed video size is wanted, but this size is also
>> likely not to match HW requirement. If hw_converter decide to give back
>> something smaller, there is nothing image_generator can do. It would
>> have to try again with random size to find out that best match. It's a
>> bit silly to force that on application, as the hw_converter know the
>> closest best match, which is simply the next valid bigger size if that
>> exist.
>>
>> hope that helps,
>> Nicolas
>
> Nicolas, thank you for shedding light on this problem. I see that it is
> not MFC specific. It seems that the problem applies to all Video4Linux2
> devices that use v4l_bound_align_image. I agree with you that this deservers
> an RFC and some discussion as this would change the behaviour of quite
> a few drivers.
>
> I think the documentation does not specify how TRY_FMT/S_FMT should adjust
> the parameters. Maybe it would a good idea to add some flagS that determine
> the behaviour?

I think we should add flags here as well. NEAREST (the default), ROUND_DOWN and
ROUND_UP. Existing calls will use NEAREST. I can think of use-cases for all
three of these, and I think the caller should just have to specify what is
needed.

Just replacing the algorithm used seems asking for problems, you want to be
able to select what you want to do.

Regards,

	Hans