linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] venus: vdec: fix format enumeration
@ 2018-03-13  9:11 Alexandre Courbot
  2018-03-15  9:52 ` Stanimir Varbanov
  0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Courbot @ 2018-03-13  9:11 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Alexandre Courbot

find_format_by_index() stops enumerating formats as soon as the index
matches, and returns NULL if venus_helper_check_codec() finds out that
the format is not supported. This prevents formats to be properly
enumerated if a non-supported format is present, as the enumeration will
end with it.

Fix this by moving the call to venus_helper_check_codec() into the loop,
and keep enumerating when it fails.

Signed-off-by: Alexandre Courbot <acourbot@chromium.org>

Change-Id: I4ff66e0b85172598efa59a6f01da8cb60597a6a5
---
 drivers/media/platform/qcom/venus/vdec.c | 13 +++++++------
 drivers/media/platform/qcom/venus/venc.c |  9 +++++++--
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index c9e9576bb08a..3677302cfe43 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -135,20 +135,21 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
 		return NULL;
 
 	for (i = 0; i < size; i++) {
+		bool valid;
+
 		if (fmt[i].type != type)
 			continue;
-		if (k == index)
+		valid = (type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ||
+			 venus_helper_check_codec(inst, fmt[i].pixfmt));
+		if (k == index && valid)
 			break;
-		k++;
+		if (valid)
+			k++;
 	}
 
 	if (i == size)
 		return NULL;
 
-	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
-	    !venus_helper_check_codec(inst, fmt[i].pixfmt))
-		return NULL;
-
 	return &fmt[i];
 }
 
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index e3a10a852cad..5eba4c7cd52e 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -120,11 +120,16 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
 		return NULL;
 
 	for (i = 0; i < size; i++) {
+		bool valid;
+
 		if (fmt[i].type != type)
 			continue;
-		if (k == index)
+		valid = (type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
+			 venus_helper_check_codec(inst, fmt[i].pixfmt));
+		if (k == index && valid)
 			break;
-		k++;
+		if (valid)
+			k++;
 	}
 
 	if (i == size)
-- 
2.16.2.660.g709887971b-goog

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] venus: vdec: fix format enumeration
  2018-03-13  9:11 [PATCH] venus: vdec: fix format enumeration Alexandre Courbot
@ 2018-03-15  9:52 ` Stanimir Varbanov
  2018-03-19  9:29   ` Alexandre Courbot
  0 siblings, 1 reply; 3+ messages in thread
From: Stanimir Varbanov @ 2018-03-15  9:52 UTC (permalink / raw)
  To: Alexandre Courbot; +Cc: linux-media, linux-arm-msm, linux-kernel

Hi Alex,

Thanks for the patch!

On 13.03.2018 11:11, Alexandre Courbot wrote:
> find_format_by_index() stops enumerating formats as soon as the index
> matches, and returns NULL if venus_helper_check_codec() finds out that
> the format is not supported. This prevents formats to be properly
> enumerated if a non-supported format is present, as the enumeration will
> end with it.

Please add fixes tag,

Fixes: 29f0133ec6 media: venus: use helper function to check supported
codecs

> 
> Fix this by moving the call to venus_helper_check_codec() into the loop,
> and keep enumerating when it fails.

Good catch!

> 
> Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
> 
> Change-Id: I4ff66e0b85172598efa59a6f01da8cb60597a6a5

You forgot to delete gerrit id.

> ---
>   drivers/media/platform/qcom/venus/vdec.c | 13 +++++++------
>   drivers/media/platform/qcom/venus/venc.c |  9 +++++++--
>   2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
> index c9e9576bb08a..3677302cfe43 100644
> --- a/drivers/media/platform/qcom/venus/vdec.c
> +++ b/drivers/media/platform/qcom/venus/vdec.c
> @@ -135,20 +135,21 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
>   		return NULL;
>   
>   	for (i = 0; i < size; i++) {
> +		bool valid;
> +
>   		if (fmt[i].type != type)
>   			continue;
> -		if (k == index)
> +		valid = (type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ||
> +			 venus_helper_check_codec(inst, fmt[i].pixfmt));

open and close braces are not needed.

> +		if (k == index && valid)
>   			break;
> -		k++;
> +		if (valid)
> +			k++;
>   	}
>   
>   	if (i == size)
>   		return NULL;
>   
> -	if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
> -	    !venus_helper_check_codec(inst, fmt[i].pixfmt))
> -		return NULL;
> -
>   	return &fmt[i];
>   }
>   
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index e3a10a852cad..5eba4c7cd52e 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -120,11 +120,16 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
>   		return NULL;
>   
>   	for (i = 0; i < size; i++) {
> +		bool valid;
> +
>   		if (fmt[i].type != type)
>   			continue;
> -		if (k == index)
> +		valid = (type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
> +			 venus_helper_check_codec(inst, fmt[i].pixfmt));
> +		if (k == index && valid)
>   			break;
> -		k++;
> +		if (valid)
> +			k++;
>   	}
>   
>   	if (i == size)

maybe we should delete the condition:

if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
!venus_helper_check_codec(inst, fmt[i].pixfmt))

as we do for the decoder?

regards,
Stan

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] venus: vdec: fix format enumeration
  2018-03-15  9:52 ` Stanimir Varbanov
@ 2018-03-19  9:29   ` Alexandre Courbot
  0 siblings, 0 replies; 3+ messages in thread
From: Alexandre Courbot @ 2018-03-19  9:29 UTC (permalink / raw)
  To: Stanimir Varbanov; +Cc: Linux Media Mailing List, linux-arm-msm, LKML

On Thu, Mar 15, 2018 at 6:52 PM, Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
> Hi Alex,
>
> Thanks for the patch!
>
> On 13.03.2018 11:11, Alexandre Courbot wrote:
>> find_format_by_index() stops enumerating formats as soon as the index
>> matches, and returns NULL if venus_helper_check_codec() finds out that
>> the format is not supported. This prevents formats to be properly
>> enumerated if a non-supported format is present, as the enumeration will
>> end with it.
>
> Please add fixes tag,
>
> Fixes: 29f0133ec6 media: venus: use helper function to check supported
> codecs

Sure.

>
>>
>> Fix this by moving the call to venus_helper_check_codec() into the loop,
>> and keep enumerating when it fails.
>
> Good catch!
>
>>
>> Signed-off-by: Alexandre Courbot <acourbot@chromium.org>
>>
>> Change-Id: I4ff66e0b85172598efa59a6f01da8cb60597a6a5
>
> You forgot to delete gerrit id.

D'oh, I should know better than send patches from my Chrome OS tree...

>
>> ---
>>   drivers/media/platform/qcom/venus/vdec.c | 13 +++++++------
>>   drivers/media/platform/qcom/venus/venc.c |  9 +++++++--
>>   2 files changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
>> index c9e9576bb08a..3677302cfe43 100644
>> --- a/drivers/media/platform/qcom/venus/vdec.c
>> +++ b/drivers/media/platform/qcom/venus/vdec.c
>> @@ -135,20 +135,21 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
>>               return NULL;
>>
>>       for (i = 0; i < size; i++) {
>> +             bool valid;
>> +
>>               if (fmt[i].type != type)
>>                       continue;
>> -             if (k == index)
>> +             valid = (type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE ||
>> +                      venus_helper_check_codec(inst, fmt[i].pixfmt));
>
> open and close braces are not needed.

Removed them.

>
>> +             if (k == index && valid)
>>                       break;
>> -             k++;
>> +             if (valid)
>> +                     k++;
>>       }
>>
>>       if (i == size)
>>               return NULL;
>>
>> -     if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
>> -         !venus_helper_check_codec(inst, fmt[i].pixfmt))
>> -             return NULL;
>> -
>>       return &fmt[i];
>>   }
>>
>> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
>> index e3a10a852cad..5eba4c7cd52e 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -120,11 +120,16 @@ find_format_by_index(struct venus_inst *inst, unsigned int index, u32 type)
>>               return NULL;
>>
>>       for (i = 0; i < size; i++) {
>> +             bool valid;
>> +
>>               if (fmt[i].type != type)
>>                       continue;
>> -             if (k == index)
>> +             valid = (type != V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
>> +                      venus_helper_check_codec(inst, fmt[i].pixfmt));
>> +             if (k == index && valid)
>>                       break;
>> -             k++;
>> +             if (valid)
>> +                     k++;
>>       }
>>
>>       if (i == size)
>
> maybe we should delete the condition:
>
> if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
> !venus_helper_check_codec(inst, fmt[i].pixfmt))
>
> as we do for the decoder?

We should indeed, missed that.

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-19  9:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13  9:11 [PATCH] venus: vdec: fix format enumeration Alexandre Courbot
2018-03-15  9:52 ` Stanimir Varbanov
2018-03-19  9:29   ` Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).