On Mon, Sep 9, 2013 at 4:52 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: > On 09/09/2013 05:48 AM, Pawel Osciak wrote: >> Hi Hans, >> Thanks for the comments, one question inline. >> >> On Fri, Aug 30, 2013 at 3:48 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote: >>> On 08/30/2013 04:17 AM, Pawel Osciak wrote: >>>> Add defines for controls found in UVC 1.5 encoding cameras. >>>> >>>> Signed-off-by: Pawel Osciak <posciak@xxxxxxxxxxxx> >>>> --- >>>> drivers/media/v4l2-core/v4l2-ctrls.c | 29 +++++++++++++++++++++++++++++ >>>> include/uapi/linux/v4l2-controls.h | 31 +++++++++++++++++++++++++++++++ >>>> 2 files changed, 60 insertions(+) >>>> >>>> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c >>>> index c3f0803..0b3a632 100644 >>>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c >>>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c >>>> @@ -781,6 +781,35 @@ const char *v4l2_ctrl_get_name(u32 id) >>>> case V4L2_CID_AUTO_FOCUS_STATUS: return "Auto Focus, Status"; >>>> case V4L2_CID_AUTO_FOCUS_RANGE: return "Auto Focus, Range"; >>>> >>>> + case V4L2_CID_ENCODER_MIN_FRAME_INTERVAL: return "Encoder, min. frame interval"; >>>> + case V4L2_CID_ENCODER_RATE_CONTROL_MODE: return "Encoder, rate control mode"; >>>> + case V4L2_CID_ENCODER_AVERAGE_BITRATE: return "Encoder, average bitrate"; >>>> + case V4L2_CID_ENCODER_CPB_SIZE: return "Encoder, CPB size"; >>>> + case V4L2_CID_ENCODER_PEAK_BIT_RATE: return "Encoder, peak bit rate"; >>>> + case V4L2_CID_ENCODER_QP_PARAM_I: return "Encoder, QP param for I frames"; >>>> + case V4L2_CID_ENCODER_QP_PARAM_P: return "Encoder, QP param for P frames"; >>>> + case V4L2_CID_ENCODER_QP_PARAM_BG: return "Encoder, QP param for B/G frames"; >>> >>> A lot of these exist already. E.g. V4L2_CID_MPEG_VIDEO_MPEG4_I/P/B_FRAME_QP. >>> >>> Samsung added support for many of these parameters for their MFC encoder (including >>> VP8 support) so you should use them as well. As mentioned in v4l2-controls.h the >>> MPEG part of the control name is historical. Interpret it as 'CODEC', not MPEG. >>> >> >> We have QP controls separately for H264, H263 and MPEG4. Why is that? >> Which one should I use for VP8? Shouldn't we unify them instead? > > I can't quite remember the details, so I've CCed Kamil since he added those controls. > At least the H264 QP controls are different from the others as they have a different > range. What's the range for VP8? > Yes, it differs, 0-127. But I feel this is pretty unfortunate, is it a good idea to multiply controls to have one per format when they have different ranges depending on the selected format in general? Perhaps a custom handler would be better? > I'm not sure why the H263/MPEG4 controls weren't unified: it might be that since the > H264 range was different we decided to split it up per codec. But I seem to remember > that there was another reason as well. > > Regards, > > Hans -- 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