Re: [RFC 07/10] sunxi-cedrus: Add a MPEG 2 codec

From: Hans Verkuil
Date: Mon Nov 14 2016 - 05:28:35 EST


On 11/14/2016 11:18 AM, Hans Verkuil wrote:
> On 08/25/2016 11:39 AM, Florent Revest wrote:
>> This patch introduces the support of MPEG2 video decoding to the
>> sunxi-cedrus video decoder driver.
>>
>> Signed-off-by: Florent Revest <florent.revest@xxxxxxxxxxxxxxxxxx>
>> ---
>> drivers/media/platform/sunxi-cedrus/Makefile | 2 +-
>> drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c | 26 +++-
>> .../platform/sunxi-cedrus/sunxi_cedrus_common.h | 2 +
>> .../media/platform/sunxi-cedrus/sunxi_cedrus_dec.c | 15 +-
>> .../media/platform/sunxi-cedrus/sunxi_cedrus_hw.c | 17 ++-
>> .../media/platform/sunxi-cedrus/sunxi_cedrus_hw.h | 4 +
>> .../platform/sunxi-cedrus/sunxi_cedrus_mpeg2.c | 152 +++++++++++++++++++++
>> 7 files changed, 211 insertions(+), 7 deletions(-)
>> create mode 100644 drivers/media/platform/sunxi-cedrus/sunxi_cedrus_mpeg2.c
>>
>> diff --git a/drivers/media/platform/sunxi-cedrus/Makefile b/drivers/media/platform/sunxi-cedrus/Makefile
>> index 14c2f7a..2d495a2 100644
>> --- a/drivers/media/platform/sunxi-cedrus/Makefile
>> +++ b/drivers/media/platform/sunxi-cedrus/Makefile
>> @@ -1,2 +1,2 @@
>> obj-$(CONFIG_VIDEO_SUNXI_CEDRUS) += sunxi_cedrus.o sunxi_cedrus_hw.o \
>> - sunxi_cedrus_dec.o
>> + sunxi_cedrus_dec.o sunxi_cedrus_mpeg2.o
>> diff --git a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c
>> index 17af34c..d1c957a 100644
>> --- a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c
>> +++ b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c
>> @@ -46,14 +46,31 @@ static int sunxi_cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
>> struct sunxi_cedrus_ctx *ctx =
>> container_of(ctrl->handler, struct sunxi_cedrus_ctx, hdl);
>>
>> - v4l2_err(&ctx->dev->v4l2_dev, "Invalid control\n");
>> - return -EINVAL;
>> + switch (ctrl->id) {
>> + case V4L2_CID_MPEG_VIDEO_MPEG2_FRAME_HDR:
>> + /* This is kept in memory and used directly. */
>> + break;
>> + default:
>> + v4l2_err(&ctx->dev->v4l2_dev, "Invalid control\n");
>
> Drop this, it's pointless since this cannot happen, and even if it could, there
> is nothing wrong about userspace passing an unknown control, that should just result in
> -EINVAL.
>

Just to be clear: 'this' == the v4l2_err call.

Regards,

Hans