Codec controls question

Codec controls question

[Kamil Debski]

Hi,

Some time ago we were discussing the set of controls that should be implemented
for codec support.

I remember that the result of this discussion was that the controls should be as
"integrated" as possible. This included the V4L2_CID_MPEG_LEVEL and all controls
related to the quantization parameter.
The problem with such approach is that the levels are different for MPEG4, H264
and H263. Same for quantization parameter - it ranges from 1 to 31 for MPEG4/H263
and from 0 to 51 for H264.

Having single controls for the more than one codec seemed as a good solution.
Unfortunately I don't see a good option to implement it, especially with the
control framework. My idea was to have the min/max values for QP set in the S_FMT
call on the CAPTURE. For MPEG_LEVEL it would be checked in the S_CTRL callback
and if it did not fit the chosen format it failed.

So I see three solutions to this problem and I wanted to ask about your opinion.

1) Have a separate controls whenever the range or valid value range differs.

This is the simplest and in my opinion the best solution I can think of. This way
we'll have different set of controls if the valid values are different (e.g.
V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
User can set the controls at any time. The only con of this approach is having
more controls.

2) Permit the user to set the control only after running S_FMT on the CAPTURE.
This approach would enable us to keep less controls, but would require to set the
min/max values for controls in the S_FMT. This could be done by adding controls
in S_FMT or by manipulating their range and disabling unused controls. In case of
MPEG_LEVEL it would require s_ctrl callback to check whether the requested level
is valid for the chosen codec.

This would be somehow against the spec, but if we allow the "codec interface" to
have some differences this would be ok.

3) Let the user set the controls whenever and check them during the STREAMON
call. 

The controls could be set anytime, and the control range supplied to the control
framework would cover values possible for all supported codecs.

This approach is more difficult than first approach. It is worse in case of user
space than the second approach - the user is unaware of any mistakes until the
STREAMON call. The argument for this approach is the possibility to have a few
controls less.

So I would like to hear a comment about the above propositions. Personally I
would opt for the first solution.

Best regards,
--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

Re: Codec controls question

[Laurent Pinchart]

Hi Kamil,

On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
> Hi,
> 
> Some time ago we were discussing the set of controls that should be
> implemented for codec support.
> 
> I remember that the result of this discussion was that the controls should
> be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL and
> all controls related to the quantization parameter.
> The problem with such approach is that the levels are different for MPEG4,
> H264 and H263. Same for quantization parameter - it ranges from 1 to 31
> for MPEG4/H263 and from 0 to 51 for H264.
> 
> Having single controls for the more than one codec seemed as a good
> solution. Unfortunately I don't see a good option to implement it,
> especially with the control framework. My idea was to have the min/max
> values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
> would be checked in the S_CTRL callback and if it did not fit the chosen
> format it failed.
> 
> So I see three solutions to this problem and I wanted to ask about your
> opinion.
> 
> 1) Have a separate controls whenever the range or valid value range
> differs.
> 
> This is the simplest and in my opinion the best solution I can think of.
> This way we'll have different set of controls if the valid values are
> different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
> User can set the controls at any time. The only con of this approach is
> having more controls.
> 
> 2) Permit the user to set the control only after running S_FMT on the
> CAPTURE. This approach would enable us to keep less controls, but would
> require to set the min/max values for controls in the S_FMT. This could be
> done by adding controls in S_FMT or by manipulating their range and
> disabling unused controls. In case of MPEG_LEVEL it would require s_ctrl
> callback to check whether the requested level is valid for the chosen
> codec.
> 
> This would be somehow against the spec, but if we allow the "codec
> interface" to have some differences this would be ok.
> 
> 3) Let the user set the controls whenever and check them during the
> STREAMON call.
> 
> The controls could be set anytime, and the control range supplied to the
> control framework would cover values possible for all supported codecs.
> 
> This approach is more difficult than first approach. It is worse in case of
> user space than the second approach - the user is unaware of any mistakes
> until the STREAMON call. The argument for this approach is the possibility
> to have a few controls less.
> 
> So I would like to hear a comment about the above propositions. Personally
> I would opt for the first solution.

I think the question boils down to whether we want to support controls that 
have different valid ranges depending on formats, or even other controls. I 
think the issue isn't specific to codoc controls.

-- 
Regards,

Laurent Pinchart

RE: Codec controls question

[Kamil Debski]

> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> Sent: 18 May 2011 16:10
> Subject: Re: Codec controls question
> 
> On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
> > Hi,

Hi,

> >
> > Some time ago we were discussing the set of controls that should be
> > implemented for codec support.
> >
> > I remember that the result of this discussion was that the controls should
> > be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL and
> > all controls related to the quantization parameter.
> > The problem with such approach is that the levels are different for MPEG4,
> > H264 and H263. Same for quantization parameter - it ranges from 1 to 31
> > for MPEG4/H263 and from 0 to 51 for H264.
> >
> > Having single controls for the more than one codec seemed as a good
> > solution. Unfortunately I don't see a good option to implement it,
> > especially with the control framework. My idea was to have the min/max
> > values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
> > would be checked in the S_CTRL callback and if it did not fit the chosen
> > format it failed.
> >
> > So I see three solutions to this problem and I wanted to ask about your
> > opinion.
> >
> > 1) Have a separate controls whenever the range or valid value range
> > differs.
> >
> > This is the simplest and in my opinion the best solution I can think of.
> > This way we'll have different set of controls if the valid values are
> > different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
> > User can set the controls at any time. The only con of this approach is
> > having more controls.
> >
> > 2) Permit the user to set the control only after running S_FMT on the
> > CAPTURE. This approach would enable us to keep less controls, but would
> > require to set the min/max values for controls in the S_FMT. This could be
> > done by adding controls in S_FMT or by manipulating their range and
> > disabling unused controls. In case of MPEG_LEVEL it would require s_ctrl
> > callback to check whether the requested level is valid for the chosen
> > codec.
> >
> > This would be somehow against the spec, but if we allow the "codec
> > interface" to have some differences this would be ok.
> >
> > 3) Let the user set the controls whenever and check them during the
> > STREAMON call.
> >
> > The controls could be set anytime, and the control range supplied to the
> > control framework would cover values possible for all supported codecs.
> >
> > This approach is more difficult than first approach. It is worse in case
> of
> > user space than the second approach - the user is unaware of any mistakes
> > until the STREAMON call. The argument for this approach is the possibility
> > to have a few controls less.
> >
> > So I would like to hear a comment about the above propositions. Personally
> > I would opt for the first solution.
> 
> I think the question boils down to whether we want to support controls that
> have different valid ranges depending on formats, or even other controls. I
> think the issue isn't specific to codoc controls.
> 

So what is your opinion on this? If there are more controls where the valid
range could depend on other controls or the chosen format then it might be worth
implementing such functionality. If there would be only a few such controls then
it might be better to just have separate controls (with the codec controls - only
*_MPEG_LEVEL and quantization parameter related controls would have different
valid range depending on the format).

--
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

RE: Codec controls question

[Hans Verkuil]

>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>> Sent: 18 May 2011 16:10
>> Subject: Re: Codec controls question
>>
>> On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
>> > Hi,
>
> Hi,
>
>> >
>> > Some time ago we were discussing the set of controls that should be
>> > implemented for codec support.
>> >
>> > I remember that the result of this discussion was that the controls
>> should
>> > be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL
>> and
>> > all controls related to the quantization parameter.
>> > The problem with such approach is that the levels are different for
>> MPEG4,
>> > H264 and H263. Same for quantization parameter - it ranges from 1 to
>> 31
>> > for MPEG4/H263 and from 0 to 51 for H264.
>> >
>> > Having single controls for the more than one codec seemed as a good
>> > solution. Unfortunately I don't see a good option to implement it,
>> > especially with the control framework. My idea was to have the min/max
>> > values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
>> > would be checked in the S_CTRL callback and if it did not fit the
>> chosen
>> > format it failed.
>> >
>> > So I see three solutions to this problem and I wanted to ask about
>> your
>> > opinion.
>> >
>> > 1) Have a separate controls whenever the range or valid value range
>> > differs.
>> >
>> > This is the simplest and in my opinion the best solution I can think
>> of.
>> > This way we'll have different set of controls if the valid values are
>> > different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
>> > User can set the controls at any time. The only con of this approach
>> is
>> > having more controls.
>> >
>> > 2) Permit the user to set the control only after running S_FMT on the
>> > CAPTURE. This approach would enable us to keep less controls, but
>> would
>> > require to set the min/max values for controls in the S_FMT. This
>> could be
>> > done by adding controls in S_FMT or by manipulating their range and
>> > disabling unused controls. In case of MPEG_LEVEL it would require
>> s_ctrl
>> > callback to check whether the requested level is valid for the chosen
>> > codec.
>> >
>> > This would be somehow against the spec, but if we allow the "codec
>> > interface" to have some differences this would be ok.
>> >
>> > 3) Let the user set the controls whenever and check them during the
>> > STREAMON call.
>> >
>> > The controls could be set anytime, and the control range supplied to
>> the
>> > control framework would cover values possible for all supported
>> codecs.
>> >
>> > This approach is more difficult than first approach. It is worse in
>> case
>> of
>> > user space than the second approach - the user is unaware of any
>> mistakes
>> > until the STREAMON call. The argument for this approach is the
>> possibility
>> > to have a few controls less.
>> >
>> > So I would like to hear a comment about the above propositions.
>> Personally
>> > I would opt for the first solution.
>>
>> I think the question boils down to whether we want to support controls
>> that
>> have different valid ranges depending on formats, or even other
>> controls. I
>> think the issue isn't specific to codoc controls.
>>
>
> So what is your opinion on this? If there are more controls where the
> valid
> range could depend on other controls or the chosen format then it might be
> worth
> implementing such functionality. If there would be only a few such
> controls then
> it might be better to just have separate controls (with the codec controls
> - only
> *_MPEG_LEVEL and quantization parameter related controls would have
> different
> valid range depending on the format).

I have experimented with control events to change ranges and while it can
be done technically it is in practice a bit of a mess. I think personally
it is just easier to have separate controls.

We are going to have similar problems if different video inputs are
controlled by different i2c devices with different (but partially
overlapping) controls. So switching an input also changes the controls. I
have experimented with this while working on control events and it became
very messy indeed. I won't do this for the first version of control
events.

One subtle but real problem with changing control ranges on the fly is
that it makes it next to impossible to save all control values to a file
and restore them later. That is a desirable feature that AFAIK is actually
in use already.

Regards,

        Hans

Re: Codec controls question

[Sylwester Nawrocki]

Hi Hans,

On 05/18/2011 05:22 PM, Hans Verkuil wrote:
>>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>>> Sent: 18 May 2011 16:10
>>> Subject: Re: Codec controls question
>>> On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
>>>> Hi,
>> Hi,
>>
>>>>
>>>> Some time ago we were discussing the set of controls that should be
>>>> implemented for codec support.
>>>>
>>>> I remember that the result of this discussion was that the controls
>>> should
>>>> be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL
>>> and
>>>> all controls related to the quantization parameter.
>>>> The problem with such approach is that the levels are different for
>>> MPEG4,
>>>> H264 and H263. Same for quantization parameter - it ranges from 1 to
>>> 31
>>>> for MPEG4/H263 and from 0 to 51 for H264.
>>>>
>>>> Having single controls for the more than one codec seemed as a good
>>>> solution. Unfortunately I don't see a good option to implement it,
>>>> especially with the control framework. My idea was to have the min/max
>>>> values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
>>>> would be checked in the S_CTRL callback and if it did not fit the
>>> chosen
>>>> format it failed.
>>>>
>>>> So I see three solutions to this problem and I wanted to ask about
>>> your
>>>> opinion.
>>>>
>>>> 1) Have a separate controls whenever the range or valid value range
>>>> differs.
>>>>
>>>> This is the simplest and in my opinion the best solution I can think
>>> of.
>>>> This way we'll have different set of controls if the valid values are
>>>> different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
>>>> User can set the controls at any time. The only con of this approach
>>> is
>>>> having more controls.
>>>>
>>>> 2) Permit the user to set the control only after running S_FMT on the
>>>> CAPTURE. This approach would enable us to keep less controls, but
>>> would
>>>> require to set the min/max values for controls in the S_FMT. This
>>> could be
>>>> done by adding controls in S_FMT or by manipulating their range and
>>>> disabling unused controls. In case of MPEG_LEVEL it would require
>>> s_ctrl
>>>> callback to check whether the requested level is valid for the chosen
>>>> codec.
>>>>
>>>> This would be somehow against the spec, but if we allow the "codec
>>>> interface" to have some differences this would be ok.
>>>>
>>>> 3) Let the user set the controls whenever and check them during the
>>>> STREAMON call.
>>>>
>>>> The controls could be set anytime, and the control range supplied to
>>> the
>>>> control framework would cover values possible for all supported
>>> codecs.
>>>>
>>>> This approach is more difficult than first approach. It is worse in
>>> case
>>> of
>>>> user space than the second approach - the user is unaware of any
>>> mistakes
>>>> until the STREAMON call. The argument for this approach is the
>>> possibility
>>>> to have a few controls less.
>>>>
>>>> So I would like to hear a comment about the above propositions.
>>> Personally
>>>> I would opt for the first solution.
>>>
>>> I think the question boils down to whether we want to support controls
>>> that
>>> have different valid ranges depending on formats, or even other
>>> controls. I
>>> think the issue isn't specific to codoc controls.
>>>
>>
>> So what is your opinion on this? If there are more controls where the
>> valid
>> range could depend on other controls or the chosen format then it might be
>> worth
>> implementing such functionality. If there would be only a few such
>> controls then
>> it might be better to just have separate controls (with the codec controls
>> - only
>> *_MPEG_LEVEL and quantization parameter related controls would have
>> different
>> valid range depending on the format).
> 
> I have experimented with control events to change ranges and while it can
> be done technically it is in practice a bit of a mess. I think personally
> it is just easier to have separate controls.
> 
> We are going to have similar problems if different video inputs are
> controlled by different i2c devices with different (but partially
> overlapping) controls. So switching an input also changes the controls. I
> have experimented with this while working on control events and it became
> very messy indeed. I won't do this for the first version of control
> events.
> 
> One subtle but real problem with changing control ranges on the fly is
> that it makes it next to impossible to save all control values to a file
> and restore them later. That is a desirable feature that AFAIK is actually
> in use already.

What are your views on creating controls in subdev s_power operation ?
Some sensors/ISPs have control ranges dependant on a firmware revision.
So before creating the controls min/max/step values need to be read from them
over I2C. We chose to postpone enabling ISP's power until a corresponding video
(or subdev) device node is opened. And thus controls are not created during
driver probing, because there is no enough information to do this.

I don't see a possibility for the applications to be able to access the controls
before they are created as this happens during a first device (either video
or subdev) open(). And they are destroyed only in video/subdev device relase().

Do you see any potential issues with this scheme ?

Regards,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center

Re: Codec controls question

[Hans Verkuil]

> Hi Hans,
>
> On 05/18/2011 05:22 PM, Hans Verkuil wrote:
>>>> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
>>>> Sent: 18 May 2011 16:10
>>>> Subject: Re: Codec controls question
>>>> On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
>>>>> Hi,
>>> Hi,
>>>
>>>>>
>>>>> Some time ago we were discussing the set of controls that should be
>>>>> implemented for codec support.
>>>>>
>>>>> I remember that the result of this discussion was that the controls
>>>> should
>>>>> be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL
>>>> and
>>>>> all controls related to the quantization parameter.
>>>>> The problem with such approach is that the levels are different for
>>>> MPEG4,
>>>>> H264 and H263. Same for quantization parameter - it ranges from 1 to
>>>> 31
>>>>> for MPEG4/H263 and from 0 to 51 for H264.
>>>>>
>>>>> Having single controls for the more than one codec seemed as a good
>>>>> solution. Unfortunately I don't see a good option to implement it,
>>>>> especially with the control framework. My idea was to have the
>>>>> min/max
>>>>> values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
>>>>> would be checked in the S_CTRL callback and if it did not fit the
>>>> chosen
>>>>> format it failed.
>>>>>
>>>>> So I see three solutions to this problem and I wanted to ask about
>>>> your
>>>>> opinion.
>>>>>
>>>>> 1) Have a separate controls whenever the range or valid value range
>>>>> differs.
>>>>>
>>>>> This is the simplest and in my opinion the best solution I can think
>>>> of.
>>>>> This way we'll have different set of controls if the valid values are
>>>>> different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
>>>>> User can set the controls at any time. The only con of this approach
>>>> is
>>>>> having more controls.
>>>>>
>>>>> 2) Permit the user to set the control only after running S_FMT on the
>>>>> CAPTURE. This approach would enable us to keep less controls, but
>>>> would
>>>>> require to set the min/max values for controls in the S_FMT. This
>>>> could be
>>>>> done by adding controls in S_FMT or by manipulating their range and
>>>>> disabling unused controls. In case of MPEG_LEVEL it would require
>>>> s_ctrl
>>>>> callback to check whether the requested level is valid for the chosen
>>>>> codec.
>>>>>
>>>>> This would be somehow against the spec, but if we allow the "codec
>>>>> interface" to have some differences this would be ok.
>>>>>
>>>>> 3) Let the user set the controls whenever and check them during the
>>>>> STREAMON call.
>>>>>
>>>>> The controls could be set anytime, and the control range supplied to
>>>> the
>>>>> control framework would cover values possible for all supported
>>>> codecs.
>>>>>
>>>>> This approach is more difficult than first approach. It is worse in
>>>> case
>>>> of
>>>>> user space than the second approach - the user is unaware of any
>>>> mistakes
>>>>> until the STREAMON call. The argument for this approach is the
>>>> possibility
>>>>> to have a few controls less.
>>>>>
>>>>> So I would like to hear a comment about the above propositions.
>>>> Personally
>>>>> I would opt for the first solution.
>>>>
>>>> I think the question boils down to whether we want to support controls
>>>> that
>>>> have different valid ranges depending on formats, or even other
>>>> controls. I
>>>> think the issue isn't specific to codoc controls.
>>>>
>>>
>>> So what is your opinion on this? If there are more controls where the
>>> valid
>>> range could depend on other controls or the chosen format then it might
>>> be
>>> worth
>>> implementing such functionality. If there would be only a few such
>>> controls then
>>> it might be better to just have separate controls (with the codec
>>> controls
>>> - only
>>> *_MPEG_LEVEL and quantization parameter related controls would have
>>> different
>>> valid range depending on the format).
>>
>> I have experimented with control events to change ranges and while it
>> can
>> be done technically it is in practice a bit of a mess. I think
>> personally
>> it is just easier to have separate controls.
>>
>> We are going to have similar problems if different video inputs are
>> controlled by different i2c devices with different (but partially
>> overlapping) controls. So switching an input also changes the controls.
>> I
>> have experimented with this while working on control events and it
>> became
>> very messy indeed. I won't do this for the first version of control
>> events.
>>
>> One subtle but real problem with changing control ranges on the fly is
>> that it makes it next to impossible to save all control values to a file
>> and restore them later. That is a desirable feature that AFAIK is
>> actually
>> in use already.
>
> What are your views on creating controls in subdev s_power operation ?
> Some sensors/ISPs have control ranges dependant on a firmware revision.
> So before creating the controls min/max/step values need to be read from
> them
> over I2C. We chose to postpone enabling ISP's power until a corresponding
> video
> (or subdev) device node is opened. And thus controls are not created
> during
> driver probing, because there is no enough information to do this.
>
> I don't see a possibility for the applications to be able to access the
> controls
> before they are created as this happens during a first device (either
> video
> or subdev) open(). And they are destroyed only in video/subdev device
> relase().
>
> Do you see any potential issues with this scheme ?

No, that should work fine.

Regards,

       Hans

Re: Codec controls question

[Laurent Pinchart]

Hi Sylwester,

On Wednesday 18 May 2011 17:57:37 Sylwester Nawrocki wrote:
> On 05/18/2011 05:22 PM, Hans Verkuil wrote:
> > 
> > I have experimented with control events to change ranges and while it can
> > be done technically it is in practice a bit of a mess. I think personally
> > it is just easier to have separate controls.
> > 
> > We are going to have similar problems if different video inputs are
> > controlled by different i2c devices with different (but partially
> > overlapping) controls. So switching an input also changes the controls. I
> > have experimented with this while working on control events and it became
> > very messy indeed. I won't do this for the first version of control
> > events.
> > 
> > One subtle but real problem with changing control ranges on the fly is
> > that it makes it next to impossible to save all control values to a file
> > and restore them later. That is a desirable feature that AFAIK is
> > actually in use already.
> 
> What are your views on creating controls in subdev s_power operation ?
> Some sensors/ISPs have control ranges dependant on a firmware revision.
> So before creating the controls min/max/step values need to be read from
> them over I2C. We chose to postpone enabling ISP's power until a
> corresponding video (or subdev) device node is opened. And thus controls
> are not created during driver probing, because there is no enough
> information to do this.

You can power the device up during probe, read the hardware/firmware version, 
power it down and create/initialize controls depending on the retrieved 
information.

> I don't see a possibility for the applications to be able to access the
> controls before they are created as this happens during a first device
> (either video or subdev) open(). And they are destroyed only in
> video/subdev device relase().
> 
> Do you see any potential issues with this scheme ?

-- 
Regards,

Laurent Pinchart

RE: Codec controls question

[Kamil Debski]

 -----Original Message-----
> From: Hans Verkuil [mailto:hverkuil@xs4all.nl]
> Sent: 18 May 2011 17:23
> To: Kamil Debski
> Cc: 'Laurent Pinchart'; linux-media@vger.kernel.org; hansverk@cisco.com;
> Marek Szyprowski
> Subject: RE: Codec controls question
> 
> >> From: Laurent Pinchart [mailto:laurent.pinchart@ideasonboard.com]
> >> Sent: 18 May 2011 16:10
> >> Subject: Re: Codec controls question
> >>
> >> On Tuesday 17 May 2011 18:23:19 Kamil Debski wrote:
> >> > Hi,
> >
> > Hi,
> >
> >> >
> >> > Some time ago we were discussing the set of controls that should be
> >> > implemented for codec support.
> >> >
> >> > I remember that the result of this discussion was that the controls
> >> should
> >> > be as "integrated" as possible. This included the V4L2_CID_MPEG_LEVEL
> >> and
> >> > all controls related to the quantization parameter.
> >> > The problem with such approach is that the levels are different for
> >> MPEG4,
> >> > H264 and H263. Same for quantization parameter - it ranges from 1 to
> >> 31
> >> > for MPEG4/H263 and from 0 to 51 for H264.
> >> >
> >> > Having single controls for the more than one codec seemed as a good
> >> > solution. Unfortunately I don't see a good option to implement it,
> >> > especially with the control framework. My idea was to have the min/max
> >> > values for QP set in the S_FMT call on the CAPTURE. For MPEG_LEVEL it
> >> > would be checked in the S_CTRL callback and if it did not fit the
> >> chosen
> >> > format it failed.
> >> >
> >> > So I see three solutions to this problem and I wanted to ask about
> >> your
> >> > opinion.
> >> >
> >> > 1) Have a separate controls whenever the range or valid value range
> >> > differs.
> >> >
> >> > This is the simplest and in my opinion the best solution I can think
> >> of.
> >> > This way we'll have different set of controls if the valid values are
> >> > different (e.g. V4L2_CID_MPEG_MPEG4_LEVEL, V4L2_CID_MPEG_H264_LEVEL).
> >> > User can set the controls at any time. The only con of this approach
> >> is
> >> > having more controls.
> >> >
> >> > 2) Permit the user to set the control only after running S_FMT on the
> >> > CAPTURE. This approach would enable us to keep less controls, but
> >> would
> >> > require to set the min/max values for controls in the S_FMT. This
> >> could be
> >> > done by adding controls in S_FMT or by manipulating their range and
> >> > disabling unused controls. In case of MPEG_LEVEL it would require
> >> s_ctrl
> >> > callback to check whether the requested level is valid for the chosen
> >> > codec.
> >> >
> >> > This would be somehow against the spec, but if we allow the "codec
> >> > interface" to have some differences this would be ok.
> >> >
> >> > 3) Let the user set the controls whenever and check them during the
> >> > STREAMON call.
> >> >
> >> > The controls could be set anytime, and the control range supplied to
> >> the
> >> > control framework would cover values possible for all supported
> >> codecs.
> >> >
> >> > This approach is more difficult than first approach. It is worse in
> >> case
> >> of
> >> > user space than the second approach - the user is unaware of any
> >> mistakes
> >> > until the STREAMON call. The argument for this approach is the
> >> possibility
> >> > to have a few controls less.
> >> >
> >> > So I would like to hear a comment about the above propositions.
> >> Personally
> >> > I would opt for the first solution.
> >>
> >> I think the question boils down to whether we want to support controls
> >> that
> >> have different valid ranges depending on formats, or even other
> >> controls. I
> >> think the issue isn't specific to codoc controls.
> >>
> >
> > So what is your opinion on this? If there are more controls where the
> > valid
> > range could depend on other controls or the chosen format then it might be
> > worth
> > implementing such functionality. If there would be only a few such
> > controls then
> > it might be better to just have separate controls (with the codec controls
> > - only
> > *_MPEG_LEVEL and quantization parameter related controls would have
> > different
> > valid range depending on the format).
> 
> I have experimented with control events to change ranges and while it can
> be done technically it is in practice a bit of a mess. I think personally
> it is just easier to have separate controls.
> 
> We are going to have similar problems if different video inputs are
> controlled by different i2c devices with different (but partially
> overlapping) controls. So switching an input also changes the controls. I
> have experimented with this while working on control events and it became
> very messy indeed. I won't do this for the first version of control
> events.
> 
> One subtle but real problem with changing control ranges on the fly is
> that it makes it next to impossible to save all control values to a file
> and restore them later. That is a desirable feature that AFAIK is actually
> in use already.
> 

Thanks for your comment. I will go for implementing separate controls as I also
find having dynamic ranges a bit messy.

Best regards,
-- 
Kamil Debski
Linux Platform Group
Samsung Poland R&D Center

Re: Codec controls question

[Sylwester Nawrocki]

Hi Laurent,

On 05/18/2011 06:03 PM, Laurent Pinchart wrote:
> On Wednesday 18 May 2011 17:57:37 Sylwester Nawrocki wrote:
>> On 05/18/2011 05:22 PM, Hans Verkuil wrote:
>>>
>>> I have experimented with control events to change ranges and while it can
>>> be done technically it is in practice a bit of a mess. I think personally
>>> it is just easier to have separate controls.
>>>
>>> We are going to have similar problems if different video inputs are
>>> controlled by different i2c devices with different (but partially
>>> overlapping) controls. So switching an input also changes the controls. I
>>> have experimented with this while working on control events and it became
>>> very messy indeed. I won't do this for the first version of control
>>> events.
>>>
>>> One subtle but real problem with changing control ranges on the fly is
>>> that it makes it next to impossible to save all control values to a file
>>> and restore them later. That is a desirable feature that AFAIK is
>>> actually in use already.
>>
>> What are your views on creating controls in subdev s_power operation ?
>> Some sensors/ISPs have control ranges dependant on a firmware revision.
>> So before creating the controls min/max/step values need to be read from
>> them over I2C. We chose to postpone enabling ISP's power until a
>> corresponding video (or subdev) device node is opened. And thus controls
>> are not created during driver probing, because there is no enough
>> information to do this.
> 
> You can power the device up during probe, read the hardware/firmware version, 
> power it down and create/initialize controls depending on the retrieved 
> information.

Yes, I suppose this is what all drivers should normally do. But if for example
there are 2 sensor's registered during a media device initialization and it takes
about 100ms and 600 ms to initialize each one respectively, then if the driver
is compiled in the kernel the system boot time would increase by 700ms.   
If the whole driver is compiled as a LKM this could be acceptable though.

I'm still not convinced, the most straightforward method would be to power up
the sensor in probe(), but there comes that unfortunate delay. 

> 
>> I don't see a possibility for the applications to be able to access the
>> controls before they are created as this happens during a first device
>> (either video or subdev) open(). And they are destroyed only in
>> video/subdev device relase().
>>
>> Do you see any potential issues with this scheme ?
> 

Thanks,
-- 
Sylwester Nawrocki
Samsung Poland R&D Center