Message ID | HE1PR0301MB215459BE4AF81C7678900B638F4F9@HE1PR0301MB2154.eurprd03.prod.outlook.com |
---|---|
State | Accepted |
Commit | 4e39cd67b7275dc525684e54d0bd651ef4ee149b |
Headers | show |
Series | [FFmpeg-devel] fftools/ffmpeg_filter: Fix check for mjpeg encoder | 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 |
Fixes #9186 for me. > -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Andreas Rheinhardt > Sent: Tuesday, April 13, 2021 10:32 AM > To: ffmpeg-devel@ffmpeg.org > Cc: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > Subject: [FFmpeg-devel] [PATCH] fftools/ffmpeg_filter: Fix check for mjpeg encoder > > The MJPEG encoder supports some pixel format/color range combinations > only when strictness is set to unofficial or less. Before commit > 059fc2d9da5364627613fb3e6424079e14dbdfd3 said encoder's pix_fmts array > only included the pixel formats supported with default strictness. > When strictness was <= unofficial, fftools/ffmpeg_filter.c used > an extended list of pixel formats instead of the encoder's including > the pixel formats only supported when strictness <= unofficial. > > Said commit turned the logic around: The encoder's pix_fmts array now > included all pixel formats and fftools/ffmpeg_filter.c instead used > a small list of all pixel formats supported when strictness is > > unofficial and the encoder's pixel formats instead. In particular, > the codec's pix_fmt is not used when strictness is normal. > > This works for the mjpeg encoder; yet it did not work for other > (hardware-based) mjpeg encoders, because the check for whether one is > using the MJPEG encoder is wrong: It just checks the codec id. > So if one used strict unofficial with a hardware-accelerated MJPEG > encoder before commit 059fc2d9da53, the unofficial (non-hardware) > pixel formats of the MJPEG encoder would be used; since said commit > the codec's pixel formats are overridden at ordinary strictness > by the ordinary MJPEG pixel formats. This leads to format conversion > errors lateron which were reported in #9186. > > The solution to this is to check AVCodec.name instead of its id. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > fftools/ffmpeg_filter.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c > index 400f5a4188..e7c05eb3f9 100644 > --- a/fftools/ffmpeg_filter.c > +++ b/fftools/ffmpeg_filter.c > @@ -41,13 +41,13 @@ > > // FIXME: YUV420P etc. are actually supported with full color range, > // yet the latter information isn't available here. > -static const enum AVPixelFormat *get_compliance_normal_pix_fmts(enum AVCodecID codec_id, const enum AVPixelFormat > default_formats[]) > +static const enum AVPixelFormat *get_compliance_normal_pix_fmts(const AVCodec *codec, const enum AVPixelFormat > default_formats[]) > { > static const enum AVPixelFormat mjpeg_formats[] = > { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P, > AV_PIX_FMT_NONE }; > > - if (codec_id == AV_CODEC_ID_MJPEG) { > + if (!strcmp(codec->name, "mjpeg")) { > return mjpeg_formats; > } else { > return default_formats; > @@ -65,7 +65,7 @@ static enum AVPixelFormat choose_pixel_fmt(AVStream *st, AVCodecContext *enc_ctx > enum AVPixelFormat best= AV_PIX_FMT_NONE; > > if (enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) { > - p = get_compliance_normal_pix_fmts(enc_ctx->codec_id, p); > + p = get_compliance_normal_pix_fmts(codec, p); > } > for (; *p != AV_PIX_FMT_NONE; p++) { > best = av_find_best_pix_fmt_of_2(best, *p, target, has_alpha, NULL); > @@ -113,7 +113,7 @@ static char *choose_pix_fmts(OutputFilter *ofilter) > > p = ost->enc->pix_fmts; > if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) { > - p = get_compliance_normal_pix_fmts(ost->enc_ctx->codec_id, p); > + p = get_compliance_normal_pix_fmts(ost->enc, p); > } > > for (; *p != AV_PIX_FMT_NONE; p++) { > -- > 2.27.0 > > _______________________________________________ > 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 --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c index 400f5a4188..e7c05eb3f9 100644 --- a/fftools/ffmpeg_filter.c +++ b/fftools/ffmpeg_filter.c @@ -41,13 +41,13 @@ // FIXME: YUV420P etc. are actually supported with full color range, // yet the latter information isn't available here. -static const enum AVPixelFormat *get_compliance_normal_pix_fmts(enum AVCodecID codec_id, const enum AVPixelFormat default_formats[]) +static const enum AVPixelFormat *get_compliance_normal_pix_fmts(const AVCodec *codec, const enum AVPixelFormat default_formats[]) { static const enum AVPixelFormat mjpeg_formats[] = { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_NONE }; - if (codec_id == AV_CODEC_ID_MJPEG) { + if (!strcmp(codec->name, "mjpeg")) { return mjpeg_formats; } else { return default_formats; @@ -65,7 +65,7 @@ static enum AVPixelFormat choose_pixel_fmt(AVStream *st, AVCodecContext *enc_ctx enum AVPixelFormat best= AV_PIX_FMT_NONE; if (enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) { - p = get_compliance_normal_pix_fmts(enc_ctx->codec_id, p); + p = get_compliance_normal_pix_fmts(codec, p); } for (; *p != AV_PIX_FMT_NONE; p++) { best = av_find_best_pix_fmt_of_2(best, *p, target, has_alpha, NULL); @@ -113,7 +113,7 @@ static char *choose_pix_fmts(OutputFilter *ofilter) p = ost->enc->pix_fmts; if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) { - p = get_compliance_normal_pix_fmts(ost->enc_ctx->codec_id, p); + p = get_compliance_normal_pix_fmts(ost->enc, p); } for (; *p != AV_PIX_FMT_NONE; p++) {
The MJPEG encoder supports some pixel format/color range combinations only when strictness is set to unofficial or less. Before commit 059fc2d9da5364627613fb3e6424079e14dbdfd3 said encoder's pix_fmts array only included the pixel formats supported with default strictness. When strictness was <= unofficial, fftools/ffmpeg_filter.c used an extended list of pixel formats instead of the encoder's including the pixel formats only supported when strictness <= unofficial. Said commit turned the logic around: The encoder's pix_fmts array now included all pixel formats and fftools/ffmpeg_filter.c instead used a small list of all pixel formats supported when strictness is > unofficial and the encoder's pixel formats instead. In particular, the codec's pix_fmt is not used when strictness is normal. This works for the mjpeg encoder; yet it did not work for other (hardware-based) mjpeg encoders, because the check for whether one is using the MJPEG encoder is wrong: It just checks the codec id. So if one used strict unofficial with a hardware-accelerated MJPEG encoder before commit 059fc2d9da53, the unofficial (non-hardware) pixel formats of the MJPEG encoder would be used; since said commit the codec's pixel formats are overridden at ordinary strictness by the ordinary MJPEG pixel formats. This leads to format conversion errors lateron which were reported in #9186. The solution to this is to check AVCodec.name instead of its id. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- fftools/ffmpeg_filter.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)