| Message ID | HE1PR0301MB2154C133F0DD61FEDAB363318F709@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | Accepted |
| Commit | 33db0cbfd08384d611370006a77675cc6b778d12 |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/jpeglsenc: Remove redundant pixel format checks | expand |
| 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 |
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.
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 --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; }
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(-)