diff mbox series

[FFmpeg-devel,v3,2/2] avcodec/mediacodecdec: check ff_Build_SDK_INT return value

Message ID tencent_9E00D9699A7DFAACB638C1C2EDB56469060A@qq.com
State Accepted
Commit 9ea6d93218c7d4581584df5b5b86b5f4fe0196b1
Headers show
Series None | expand

Commit Message

Zhao Zhili Jan. 10, 2023, 11:35 a.m. UTC
From: Zhao Zhili <zhilizhao@tencent.com>

---
v3: when target API <= 24 ==> when target API < 24
v2: add comments

 libavcodec/mediacodecdec.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Tomas Härdin Jan. 10, 2023, 1:37 p.m. UTC | #1
tis 2023-01-10 klockan 19:35 +0800 skrev Zhao Zhili:
> From: Zhao Zhili <zhilizhao@tencent.com>
> 
> ---
> v3: when target API <= 24 ==> when target API < 24
> v2: add comments
> 
>  libavcodec/mediacodecdec.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
> index 11f655a9aa..21464900d1 100644
> --- a/libavcodec/mediacodecdec.c
> +++ b/libavcodec/mediacodecdec.c
> @@ -415,7 +415,13 @@ static av_cold int
> mediacodec_decode_init(AVCodecContext *avctx)
>             s->ctx->codec_name, ret);
>  
>      sdk_int = ff_Build_SDK_INT(avctx);
> -    if (sdk_int <= 23 &&
> +    /* ff_Build_SDK_INT can fail when target API < 24 and JVM isn't
> available.
> +     * If we don't check sdk_int > 0, the workaround might be
> enabled by
> +     * mistake.
> +     * JVM is required to make the workaround works reliably. On the
> other hand,
> +     * missing a workaround should not be a serious issue, we do as
> best we can.
> +     */
> +    if (sdk_int > 0 && sdk_int <= 23 &&
>          strcmp(s->ctx->codec_name,
> "OMX.amlogic.mpeg2.decoder.awesome") == 0) {
>          av_log(avctx, AV_LOG_INFO, "Enabling workaround for %s on
> API=%d\n",
>                 s->ctx->codec_name, sdk_int);

Looks OK

/Tomas
Zhao Zhili Jan. 11, 2023, 3:25 a.m. UTC | #2
> On Jan 10, 2023, at 21:37, Tomas Härdin <git@haerdin.se> wrote:
> 
> tis 2023-01-10 klockan 19:35 +0800 skrev Zhao Zhili:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> ---
>> v3: when target API <= 24 ==> when target API < 24
>> v2: add comments
>> 
>>  libavcodec/mediacodecdec.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
>> index 11f655a9aa..21464900d1 100644
>> --- a/libavcodec/mediacodecdec.c
>> +++ b/libavcodec/mediacodecdec.c
>> @@ -415,7 +415,13 @@ static av_cold int
>> mediacodec_decode_init(AVCodecContext *avctx)
>>             s->ctx->codec_name, ret);
>>  
>>      sdk_int = ff_Build_SDK_INT(avctx);
>> -    if (sdk_int <= 23 &&
>> +    /* ff_Build_SDK_INT can fail when target API < 24 and JVM isn't
>> available.
>> +     * If we don't check sdk_int > 0, the workaround might be
>> enabled by
>> +     * mistake.
>> +     * JVM is required to make the workaround works reliably. On the
>> other hand,
>> +     * missing a workaround should not be a serious issue, we do as
>> best we can.
>> +     */
>> +    if (sdk_int > 0 && sdk_int <= 23 &&
>>          strcmp(s->ctx->codec_name,
>> "OMX.amlogic.mpeg2.decoder.awesome") == 0) {
>>          av_log(avctx, AV_LOG_INFO, "Enabling workaround for %s on
>> API=%d\n",
>>                 s->ctx->codec_name, sdk_int);
> 
> Looks OK

Patchset pushed, thanks for the review.

> 
> /Tomas
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff mbox series

Patch

diff --git a/libavcodec/mediacodecdec.c b/libavcodec/mediacodecdec.c
index 11f655a9aa..21464900d1 100644
--- a/libavcodec/mediacodecdec.c
+++ b/libavcodec/mediacodecdec.c
@@ -415,7 +415,13 @@  static av_cold int mediacodec_decode_init(AVCodecContext *avctx)
            s->ctx->codec_name, ret);
 
     sdk_int = ff_Build_SDK_INT(avctx);
-    if (sdk_int <= 23 &&
+    /* ff_Build_SDK_INT can fail when target API < 24 and JVM isn't available.
+     * If we don't check sdk_int > 0, the workaround might be enabled by
+     * mistake.
+     * JVM is required to make the workaround works reliably. On the other hand,
+     * missing a workaround should not be a serious issue, we do as best we can.
+     */
+    if (sdk_int > 0 && sdk_int <= 23 &&
         strcmp(s->ctx->codec_name, "OMX.amlogic.mpeg2.decoder.awesome") == 0) {
         av_log(avctx, AV_LOG_INFO, "Enabling workaround for %s on API=%d\n",
                s->ctx->codec_name, sdk_int);