Hi, > From: Pawel Osciak [mailto:posciak@xxxxxxxxxxxx] > Sent: Monday, September 09, 2013 9:59 AM > > 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. > > We had a discussion about this on linux-media mailing list. It can be found here: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/326 06 In short, it is a mix of two reasons: one - the valid range is different for different formats and second - implementing controls which have different min/max values depending on format was not easy. On the one hand I am thinking that now, when we have more codecs, it would be better to have a single control, on the other hand what about backward compatibility? Is there a graceful way to merge H263 and H264 QP controls? Best wishes, Kamil -- 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
