Laurent Pinchart wrote:
> Hi Sakari,
>
> On Monday 28 November 2011 17:01:12 Sakari Ailus wrote:
>> On Mon, Nov 28, 2011 at 12:37:34PM +0100, Laurent Pinchart wrote:
>>> When mapping from a V4L2 pixel format to a media bus format in the
>>> VIDIOC_TRY_FMT and VIDIOC_S_FMT handlers, the requested format may be
>>> unsupported by the driver. Return a hardcoded format instead of
>>> WARN()ing in that case.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
>>> ---
>>>
>>> drivers/media/video/omap3isp/ispvideo.c | 8 ++++----
>>> 1 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/video/omap3isp/ispvideo.c
>>> b/drivers/media/video/omap3isp/ispvideo.c index d100072..ffe7ce9 100644
>>> --- a/drivers/media/video/omap3isp/ispvideo.c
>>> +++ b/drivers/media/video/omap3isp/ispvideo.c
>>> @@ -210,14 +210,14 @@ static void isp_video_pix_to_mbus(const struct
>>> v4l2_pix_format *pix,
>>>
>>> mbus->width = pix->width;
>>> mbus->height = pix->height;
>>>
>>> - for (i = 0; i < ARRAY_SIZE(formats); ++i) {
>>> + /* Skip the last format in the loop so that it will be selected if no
>>> + * match is found.
>>> + */
>>> + for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
>>>
>>> if (formats[i].pixelformat == pix->pixelformat)
>>>
>>> break;
>>>
>>> }
>>>
>>> - if (WARN_ON(i == ARRAY_SIZE(formats)))
>>> - return;
>>> -
>>>
>>> mbus->code = formats[i].code;
>>> mbus->colorspace = pix->colorspace;
>>> mbus->field = pix->field;
>>
>> In case of setting or trying an invalid format, instead of selecting a
>> default format, shouldn't we leave the format unchanced --- the current
>> setting is valid after all.
>
> TRY/SET operations must succeed. The format we select when an invalid format
> is requested isn't specified. We could keep the current format, but wouldn't
> that be more confusing for applications ? The format they would get in
> response to a TRY/SET operation would then potentially depend on the previous
> SET operations.
I don't think a change to something that has nothing to do what was
requested is better than not changing it. The application has requested
a particular format; changing it to something else isn't useful for the
application. And if the application would try more than invalid format
in a row, they both would yield to the same default format.
I would personally not change it.
What I can find in the spec is this:
"When the application calls the VIDIOC_S_FMT ioctl with a pointer to a
v4l2_format structure the driver checks and adjusts the parameters
against hardware abilities."
I wonder how other drivers behave.
--
Sakari Ailus
sakari.ailus@xxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
