Re: [PATCH 10/10] media: coda: require all decoder command flags to be cleared

From: Hans Verkuil <hverkuil@xs4all.nl>
To: Philipp Zabel <p.zabel@pengutronix.de>, linux-media@vger.kernel.org
Cc: kernel@pengutronix.de
Subject: Re: [PATCH 10/10] media: coda: require all decoder command flags to be cleared
Date: Wed, 10 Apr 2019 15:53:28 +0200	[thread overview]
Message-ID: <c8624e76-e4cf-24eb-f398-59f8cfe1e984@xs4all.nl> (raw)
In-Reply-To: <1554829061.5799.7.camel@pengutronix.de>

On 4/9/19 6:57 PM, Philipp Zabel wrote:
> On Mon, 2019-04-08 at 14:32 +0200, Philipp Zabel wrote:
>> The memory-to-memory stateful video decoder interface documentation
>> requires the decoder stop command initiating the drain sequence to have
>> flags set to zero.
>> Stop to black makes no sense as stopped memory-to-memory decoders do not
>> produce any frames, and stopping immediately can be achieved by stopping
>> the output video queue with VIDIOC_STREAMOFF.
>>
>> The mute audio start command flag serves no purpose as the coda driver
>> does not handle any audio formats, and does not support playback at
>> non-standard speeds.
>>
>> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
>> ---
>>  drivers/media/platform/coda/coda-common.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
>> index 318f0be103bb..96798f98734a 100644
>> --- a/drivers/media/platform/coda/coda-common.c
>> +++ b/drivers/media/platform/coda/coda-common.c
>> @@ -1050,10 +1050,10 @@ static int coda_try_decoder_cmd(struct file *file, void *fh,
>>  	if (dc->cmd != V4L2_DEC_CMD_STOP)
>>  		return -EINVAL;
>>  
>> -	if (dc->flags & V4L2_DEC_CMD_STOP_TO_BLACK)
>> +	if (dc->stop.pts != 0)
>>  		return -EINVAL;
>>  
>> -	if (!(dc->flags & V4L2_DEC_CMD_STOP_IMMEDIATELY) && (dc->stop.pts != 0))
>> +	if (dc->flags != 0)
>>  		return -EINVAL;
> 
> This change currently causes a v4l2-compliance failure
> 
>                 fail: v4l2-test-codecs.cpp(104): ret != 0
>         test VIDIOC_(TRY_)DECODER_CMD: FAIL
> 
> because it still expects V4L2_DEC_CMD_STOP_IMMEDIATELY to be supported.

I've fixed this in my v4l-utils work-in-progress branch:

https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=vicodec

But I want to hold off on this patch a little bit.

I think we need to add helpers for the try_en/decoder_cmd ioctls
to v4l2-mem2mem.c since it will be the same for all codecs.

I also had to fix vicodec since the checks it did weren't complete:

https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=vicodec&id=4e95bff9a15b63179fab20ac2113c238dc20665b

Regards,

	Hans