Laurent Pinchart wrote:
> Hi Sakari,
Hi Laurent,
And thanks for the comments.
> On Wednesday 06 April 2011 11:25:56 Sakari Ailus wrote:
>> Nayden Kanchev wrote:
>>> On 04/06/2011 11:10 AM, Sakari Ailus wrote:
>>>> - Added an open question on a new control:
>>>> V4L2_CID_FLASH_EXTERNAL_STROBE_WHENCE.
>>>
>>> <snip>
>>>
>>>> 2. External strobe edge / level
>>>> -------------------------------
>>>>
>>>> No use is seen currently for this, but it may well appear, and the
>>>> hardware supports this. Level based trigger should be used since it is
>>>> more precise.
>>>>
>>>> V4L2_CID_FLASH_EXTERNAL_STROBE_WHENCE
>>>>
>>>> Whether the flash controller considers external strobe as edge, when the
>>>> only limit of the strobe is the timeout on flash controller, or level,
>>>> when the flash strobe will last as long as the strobe signal, or as long
>>>> until the timeout expires.
>>>>
>>>> enum v4l2_flash_external_strobe_whence {
>>>>
>>>> V4L2_CID_FLASH_EXTERNAL_STROBE_LEVEL,
>>>> V4L2_CID_FLASH_EXTERNAL_STROBE_EDGE,
>>>>
>>>> };
>>
>> Removed "CID_":
>>
>> enum v4l2_flash_external_strobe_whence {
>> V4L2_FLASH_EXTERNAL_STROBE_LEVEL,
>> V4L2_FLASH_EXTERNAL_STROBE_EDGE,
>> };
>>
>> I guess this should be an rw menu control for LED flash?
>>
>>> I agree that control over the strobe usage (level/edge) is required.
>>> Although we have some bad experience will lack of detailed information
>>> how exactly the flash chip will use those signals.
>>>
>>> For example with AS3645A flash driver strobing by edge produced really
>>> strange flash output - light intensity was changing during the process
>>> and flash was stopped before the HW timeout.
>>>
>>> On the other hand strobing by level didn't cause problems.
>>>
>>> So even if HW supports some functionally we should prevent such
>>> malfunctioning by adding some restrictions in the board code also.
>>
>> I agree.
>>
>> The control should be probably exposed to tell which kind of
>> functionality does the flash chip provide, even if the menu has just one
>> option in it.
>>
>>> I would also rename xxx_STROBE_WHENCE to xxx_STROBE_TYPE but it is just
>>> a suggestion :)
>>
>> Sounds good to me.
>>
>> V4L2_CID_FLASH_STROBE_MODE should be renamed to
>> V4L2_CID_FLASH_STROBE_WHENCE. That proper use of whence IMO. :-)
>
> Does this really need to be exposed to userspace ? Shouldn't it just be static
> information coming from platform data ?
If the sensor is expected to set the strobe length, the value needs to
be programmed to the sensor. The sensor driver should be able to do this
as it has all the timing related information on the sensor state.
What if the user wants to expose more than one frame with flash?
The sensor should be able to export the total required exposure time of
the full frame. The exposure of each line begins at different time so
the exposure time of the full frame exceeds the exposure time of a
single pixel --- it may be almost double.
The user must be able to know that the exposure time required to expose
the frame is smaller or equal to the maximum possible exposure time of
the flash.
To the user there is no significant difference between the two. There
may be effects on the following frames beyond the one(s) exposed with flash.
I agree we could omit this, at least for now.
Regards,
--
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx
--
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
