Hi Hans, On 09/19/2016 01:15 PM, Hans Verkuil wrote: > Many of my review comments for the decoder apply to the encoder as well, > so I won't repeat those. Sure, will address them too. > > On 09/07/2016 01:37 PM, Stanimir Varbanov wrote: >> This adds encoder part of the driver plus encoder controls. >> >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >> --- >> drivers/media/platform/qcom/vidc/venc.c | 1252 +++++++++++++++++++++++++ >> drivers/media/platform/qcom/vidc/venc.h | 29 + >> drivers/media/platform/qcom/vidc/venc_ctrls.c | 396 ++++++++ >> drivers/media/platform/qcom/vidc/venc_ctrls.h | 23 + >> 4 files changed, 1700 insertions(+) >> create mode 100644 drivers/media/platform/qcom/vidc/venc.c >> create mode 100644 drivers/media/platform/qcom/vidc/venc.h >> create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.c >> create mode 100644 drivers/media/platform/qcom/vidc/venc_ctrls.h >> >> diff --git a/drivers/media/platform/qcom/vidc/venc.c b/drivers/media/platform/qcom/vidc/venc.c >> new file mode 100644 >> index 000000000000..3b65f851a807 >> --- /dev/null >> +++ b/drivers/media/platform/qcom/vidc/venc.c >> @@ -0,0 +1,1252 @@ > > <snip> > >> +static int venc_s_selection(struct file *file, void *fh, >> + struct v4l2_selection *s) >> +{ >> + return -EINVAL; >> +} > > Huh? Either remove this, or implement this correctly. OK, I cannot remember why I keep it this way, might be v4l2-compliance > > <snip> > >> +static int venc_subscribe_event(struct v4l2_fh *fh, >> + const struct v4l2_event_subscription *sub) >> +{ >> + switch (sub->type) { >> + case V4L2_EVENT_EOS: >> + return v4l2_event_subscribe(fh, sub, 2, NULL); >> + case V4L2_EVENT_SOURCE_CHANGE: >> + return v4l2_src_change_event_subscribe(fh, sub); > > These two events aren't used in this driver AFAICT, so this can be dropped. > > Since that leaves just V4L2_EVENT_CTRL this function can be replaced by > v4l2_ctrl_subscribe_event(). sure I can remove it. -- regards, Stan -- 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