diff mbox series

[FFmpeg-devel] avcodec/jpeglsenc: Remove redundant pixel format checks

Message ID HE1PR0301MB2154C133F0DD61FEDAB363318F709@HE1PR0301MB2154.eurprd03.prod.outlook.com
State Accepted
Commit 33db0cbfd08384d611370006a77675cc6b778d12
Headers show
Series [FFmpeg-devel] avcodec/jpeglsenc: Remove redundant pixel format checks
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Andreas Rheinhardt April 12, 2021, 5:07 p.m. UTC
This encoder has AVCodec.pix_fmts set, so ff_encode_preinit() already
checks for this.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Will apply tomorrow unless there are objections.

 libavcodec/jpeglsenc.c | 8 --------
 1 file changed, 8 deletions(-)

Comments

James Almer April 12, 2021, 11:39 p.m. UTC | #1
On 4/12/2021 2:07 PM, Andreas Rheinhardt wrote:
> This encoder has AVCodec.pix_fmts set, so ff_encode_preinit() already
> checks for this.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Will apply tomorrow unless there are objections.
> 
>   libavcodec/jpeglsenc.c | 8 --------
>   1 file changed, 8 deletions(-)
> 
> diff --git a/libavcodec/jpeglsenc.c b/libavcodec/jpeglsenc.c
> index 2bb6b1407a..d03ce32f41 100644
> --- a/libavcodec/jpeglsenc.c
> +++ b/libavcodec/jpeglsenc.c
> @@ -429,14 +429,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
>   FF_ENABLE_DEPRECATION_WARNINGS
>   #endif
>   
> -    if (ctx->pix_fmt != AV_PIX_FMT_GRAY8  &&
> -        ctx->pix_fmt != AV_PIX_FMT_GRAY16 &&
> -        ctx->pix_fmt != AV_PIX_FMT_RGB24  &&
> -        ctx->pix_fmt != AV_PIX_FMT_BGR24) {
> -        av_log(ctx, AV_LOG_ERROR,
> -               "Only grayscale and RGB24/BGR24 images are supported\n");
> -        return -1;
> -    }
>       return 0;
>   }

nit: The only code left in this function after this patch will be gone 
after the bump, so maybe either wrap the entire function (and the 
AVCodec initializer) with the relevant check, or postpone applying this 
patch until after the bump so you can remove the whole thing in one go.

LGTM regardless of the above.
Andreas Rheinhardt April 12, 2021, 11:45 p.m. UTC | #2
James Almer:
> On 4/12/2021 2:07 PM, Andreas Rheinhardt wrote:
>> This encoder has AVCodec.pix_fmts set, so ff_encode_preinit() already
>> checks for this.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Will apply tomorrow unless there are objections.
>>
>>   libavcodec/jpeglsenc.c | 8 --------
>>   1 file changed, 8 deletions(-)
>>
>> diff --git a/libavcodec/jpeglsenc.c b/libavcodec/jpeglsenc.c
>> index 2bb6b1407a..d03ce32f41 100644
>> --- a/libavcodec/jpeglsenc.c
>> +++ b/libavcodec/jpeglsenc.c
>> @@ -429,14 +429,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
>>   FF_ENABLE_DEPRECATION_WARNINGS
>>   #endif
>>   -    if (ctx->pix_fmt != AV_PIX_FMT_GRAY8  &&
>> -        ctx->pix_fmt != AV_PIX_FMT_GRAY16 &&
>> -        ctx->pix_fmt != AV_PIX_FMT_RGB24  &&
>> -        ctx->pix_fmt != AV_PIX_FMT_BGR24) {
>> -        av_log(ctx, AV_LOG_ERROR,
>> -               "Only grayscale and RGB24/BGR24 images are supported\n");
>> -        return -1;
>> -    }
>>       return 0;
>>   }
> 
> nit: The only code left in this function after this patch will be gone
> after the bump, so maybe either wrap the entire function (and the
> AVCodec initializer) with the relevant check, or postpone applying this
> patch until after the bump so you can remove the whole thing in one go.
> 

I am aware of that and my current plan is to just remove the whole init
function in the patch that removes the coded frame. I don't think it
makes much sense to touch the #ifs and even add new ones.

> LGTM regardless of the above.
diff mbox series

Patch

diff --git a/libavcodec/jpeglsenc.c b/libavcodec/jpeglsenc.c
index 2bb6b1407a..d03ce32f41 100644
--- a/libavcodec/jpeglsenc.c
+++ b/libavcodec/jpeglsenc.c
@@ -429,14 +429,6 @@  FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-    if (ctx->pix_fmt != AV_PIX_FMT_GRAY8  &&
-        ctx->pix_fmt != AV_PIX_FMT_GRAY16 &&
-        ctx->pix_fmt != AV_PIX_FMT_RGB24  &&
-        ctx->pix_fmt != AV_PIX_FMT_BGR24) {
-        av_log(ctx, AV_LOG_ERROR,
-               "Only grayscale and RGB24/BGR24 images are supported\n");
-        return -1;
-    }
     return 0;
 }