linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* setting volatile v4l2-control
@ 2015-01-27 13:32 Jacek Anaszewski
  2015-01-27 14:14 ` Hans Verkuil
  0 siblings, 1 reply; 3+ messages in thread
From: Jacek Anaszewski @ 2015-01-27 13:32 UTC (permalink / raw)
  To: linux-media, Hans Verkuil

While testing the LED / flash API integration patches
I noticed that the v4l2-controls marked as volatile with
V4L2_CTRL_FLAG_VOLATILE flag behave differently than I would
expect.

Let's consider following use case:

There is a volatile V4L2_CID_FLASH_INTENSITY v4l2 control with
following constraints:

min: 1
max: 100
step: 1
def: 1

1. Set the V4L2_CID_FLASH_INTENSITY control to 100.
	- as a result s_ctrl op is called
2. Set flash_brightness LED sysfs attribute to 10.
3. Set the V4L2_CID_FLASH_INTENSITY control to 100.
	- s_ctrl op isn't called

This way we are unable to write a new value to the device, despite
that the related setting was changed from the LED subsystem level.

I would expect that if a control is marked volatile, then
the v4l2-control framework should by default call g_volatile_ctrl
op before set and not try to use the cached value.

Is there some vital reason for not doing this?

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: setting volatile v4l2-control
  2015-01-27 13:32 setting volatile v4l2-control Jacek Anaszewski
@ 2015-01-27 14:14 ` Hans Verkuil
  2015-01-27 15:43   ` Jacek Anaszewski
  0 siblings, 1 reply; 3+ messages in thread
From: Hans Verkuil @ 2015-01-27 14:14 UTC (permalink / raw)
  To: Jacek Anaszewski, linux-media

On 01/27/15 14:32, Jacek Anaszewski wrote:
> While testing the LED / flash API integration patches
> I noticed that the v4l2-controls marked as volatile with
> V4L2_CTRL_FLAG_VOLATILE flag behave differently than I would
> expect.
> 
> Let's consider following use case:
> 
> There is a volatile V4L2_CID_FLASH_INTENSITY v4l2 control with
> following constraints:
> 
> min: 1
> max: 100
> step: 1
> def: 1
> 
> 1. Set the V4L2_CID_FLASH_INTENSITY control to 100.
>     - as a result s_ctrl op is called
> 2. Set flash_brightness LED sysfs attribute to 10.
> 3. Set the V4L2_CID_FLASH_INTENSITY control to 100.
>     - s_ctrl op isn't called
> 
> This way we are unable to write a new value to the device, despite
> that the related setting was changed from the LED subsystem level.
> 
> I would expect that if a control is marked volatile, then
> the v4l2-control framework should by default call g_volatile_ctrl
> op before set and not try to use the cached value.
> 
> Is there some vital reason for not doing this?

It's rather strange to have a writable volatile control. The semantics
of this are ambiguous and I don't believe we have ever used such controls
before.

Actually, the commit log of this patch (never merged) gives some
background information about this:

http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/commit/?h=volatilefix

It's never been merged because I have never been certain how to handle
such controls. Why do you have such controls in the first place? What
is it supposed to do?

Regards,

	Hans

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: setting volatile v4l2-control
  2015-01-27 14:14 ` Hans Verkuil
@ 2015-01-27 15:43   ` Jacek Anaszewski
  0 siblings, 0 replies; 3+ messages in thread
From: Jacek Anaszewski @ 2015-01-27 15:43 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media

On 01/27/2015 03:14 PM, Hans Verkuil wrote:
> On 01/27/15 14:32, Jacek Anaszewski wrote:
>> While testing the LED / flash API integration patches
>> I noticed that the v4l2-controls marked as volatile with
>> V4L2_CTRL_FLAG_VOLATILE flag behave differently than I would
>> expect.
>>
>> Let's consider following use case:
>>
>> There is a volatile V4L2_CID_FLASH_INTENSITY v4l2 control with
>> following constraints:
>>
>> min: 1
>> max: 100
>> step: 1
>> def: 1
>>
>> 1. Set the V4L2_CID_FLASH_INTENSITY control to 100.
>>      - as a result s_ctrl op is called
>> 2. Set flash_brightness LED sysfs attribute to 10.
>> 3. Set the V4L2_CID_FLASH_INTENSITY control to 100.
>>      - s_ctrl op isn't called
>>
>> This way we are unable to write a new value to the device, despite
>> that the related setting was changed from the LED subsystem level.
>>
>> I would expect that if a control is marked volatile, then
>> the v4l2-control framework should by default call g_volatile_ctrl
>> op before set and not try to use the cached value.
>>
>> Is there some vital reason for not doing this?
>
> It's rather strange to have a writable volatile control. The semantics
> of this are ambiguous and I don't believe we have ever used such controls
> before.
>
> Actually, the commit log of this patch (never merged) gives some
> background information about this:
>
> http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/commit/?h=volatilefix
>
> It's never been merged because I have never been certain how to handle
> such controls. Why do you have such controls in the first place? What
> is it supposed to do?

In case of integrated LED subsystem and V4L2 Flash API [1] a driver
can be accessed from the level of either LED subsystem sysfs interface
or v4l2-flash sub-device. Once the v4l2 sub-device is opened the LED
subsystem sysfs interface is locked, but it gets released on sub-device
closing. Since that moment the driver/device state can be changed
through sysfs interface.

When the sub-device is opened again it cannot be certain that the cached
state of the controls reflects the actual state of the driver/device.

That's why I made the shared settings volatile, maybe abusing the
intended purpose of the related flags.

[1] http://www.spinics.net/lists/linux-media/msg85351.html

-- 
Best Regards,
Jacek Anaszewski

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-01-27 15:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-27 13:32 setting volatile v4l2-control Jacek Anaszewski
2015-01-27 14:14 ` Hans Verkuil
2015-01-27 15:43   ` Jacek Anaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).