Hi Hans, On 05/18/2011 05:22 PM, Hans Verkuil wrote: >>> From: Laurent Pinchart [mailto:laurent.pinchart@xxxxxxxxxxxxxxxx] >>> 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 -- 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
