[FFmpeg-devel] [PATCH 6/9] avcodec/mjpegenc: Include all supported pix_fmts in mpegenc pix_fmts
Eoff, Ullysses A
ullysses.a.eoff at intel.com
Tue Apr 13 22:53:44 EEST 2021
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas Rheinhardt
> Sent: Tuesday, April 13, 2021 10:34 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 6/9] avcodec/mjpegenc: Include all supported pix_fmts in mpegenc pix_fmts
>
> Eoff, Ullysses A:
> > This commit breaks mjpeg_vaapi and mjpeg_qsv encoders.
> >
> > https://trac.ffmpeg.org/ticket/9186
> >
> > ...please fix.
> >
>
> Please test this patch:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2021-April/279028.html
> It should now work on all strictness levels (it was broken before said
> commit when using -strict unofficial and is broken now on default
> strictness).
>
Yes, that patch fixes it for me. Thanks for the quick response!
> - Andreas
>
> > Thanks,
> > U. Artie
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Andreas Rheinhardt
> >> Sent: Monday, April 05, 2021 10:40 PM
> >> To: ffmpeg-devel at ffmpeg.org
> >> Cc: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> Subject: [FFmpeg-devel] [PATCH 6/9] avcodec/mjpegenc: Include all supported pix_fmts in mpegenc pix_fmts
> >>
> >> Currently said list contains only the pixel formats that are always
> >> supported irrespective of the range and the value of
> >> strict_std_compliance. This makes the MJPEG encoder an outlier as all
> >> other codecs put all potentially supported pixel formats into said list
> >> and error out if the chosen pixel format is unsupported. This commit
> >> brings it therefore in line with the other encoders.
> >>
> >> The behaviour of fftools/ffmpeg_filter.c has been preserved. A more
> >> informed decision would be possible if colour range were available
> >> at this point, but it isn't.
> >>
> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> >> ---
> >> fftools/ffmpeg_filter.c | 20 +++++++-------------
> >> libavcodec/encode.c | 4 +---
> >> libavcodec/ljpegenc.c | 14 +++-----------
> >> libavcodec/mjpegenc.c | 11 ++++++++++-
> >> libavcodec/mjpegenc_common.c | 16 ++++++++++++++++
> >> libavcodec/mjpegenc_common.h | 2 ++
> >> libavcodec/mpegvideo_enc.c | 28 +---------------------------
> >> 7 files changed, 40 insertions(+), 55 deletions(-)
> >>
> >> diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
> >> index 4ab769c07b..5c44b75eff 100644
> >> --- a/fftools/ffmpeg_filter.c
> >> +++ b/fftools/ffmpeg_filter.c
> >> @@ -39,22 +39,16 @@
> >> #include "libavutil/imgutils.h"
> >> #include "libavutil/samplefmt.h"
> >>
> >> -static const enum AVPixelFormat *get_compliance_unofficial_pix_fmts(enum AVCodecID codec_id, const enum AVPixelFormat
> >> default_formats[])
> >> +// 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 mjpeg_formats[] =
> >> { AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
> >> - AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P,
> >> AV_PIX_FMT_NONE };
> >> - static const enum AVPixelFormat ljpeg_formats[] =
> >> - { AV_PIX_FMT_BGR24 , AV_PIX_FMT_BGRA , AV_PIX_FMT_BGR0,
> >> - AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_YUVJ422P,
> >> - AV_PIX_FMT_YUV420P , AV_PIX_FMT_YUV444P , AV_PIX_FMT_YUV422P,
> >> - AV_PIX_FMT_NONE};
> >>
> >> if (codec_id == AV_CODEC_ID_MJPEG) {
> >> return mjpeg_formats;
> >> - } else if (codec_id == AV_CODEC_ID_LJPEG) {
> >> - return ljpeg_formats;
> >> } else {
> >> return default_formats;
> >> }
> >> @@ -70,8 +64,8 @@ static enum AVPixelFormat choose_pixel_fmt(AVStream *st, AVCodecContext *enc_ctx
> >> int has_alpha = desc ? desc->nb_components % 2 == 0 : 0;
> >> enum AVPixelFormat best= AV_PIX_FMT_NONE;
> >>
> >> - if (enc_ctx->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL) {
> >> - p = get_compliance_unofficial_pix_fmts(enc_ctx->codec_id, p);
> >> + if (enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
> >> + p = get_compliance_normal_pix_fmts(enc_ctx->codec_id, p);
> >> }
> >> for (; *p != AV_PIX_FMT_NONE; p++) {
> >> best = av_find_best_pix_fmt_of_2(best, *p, target, has_alpha, NULL);
> >> @@ -118,8 +112,8 @@ static char *choose_pix_fmts(OutputFilter *ofilter)
> >> exit_program(1);
> >>
> >> p = ost->enc->pix_fmts;
> >> - if (ost->enc_ctx->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL) {
> >> - p = get_compliance_unofficial_pix_fmts(ost->enc_ctx->codec_id, p);
> >> + if (ost->enc_ctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
> >> + p = get_compliance_normal_pix_fmts(ost->enc_ctx->codec_id, p);
> >> }
> >>
> >> for (; *p != AV_PIX_FMT_NONE; p++) {
> >> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
> >> index 89df5235da..9a4140f91a 100644
> >> --- a/libavcodec/encode.c
> >> +++ b/libavcodec/encode.c
> >> @@ -564,9 +564,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> >> for (i = 0; avctx->codec->pix_fmts[i] != AV_PIX_FMT_NONE; i++)
> >> if (avctx->pix_fmt == avctx->codec->pix_fmts[i])
> >> break;
> >> - if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE
> >> - && !(avctx->codec_id == AV_CODEC_ID_MJPEG
> >> - && avctx->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL)) {
> >> + if (avctx->codec->pix_fmts[i] == AV_PIX_FMT_NONE) {
> >> char buf[128];
> >> snprintf(buf, sizeof(buf), "%d", avctx->pix_fmt);
> >> av_log(avctx, AV_LOG_ERROR, "Specified pixel format %s is invalid or not supported\n",
> >> diff --git a/libavcodec/ljpegenc.c b/libavcodec/ljpegenc.c
> >> index dd91c729d4..74a2cdcc46 100644
> >> --- a/libavcodec/ljpegenc.c
> >> +++ b/libavcodec/ljpegenc.c
> >> @@ -289,19 +289,11 @@ static av_cold int ljpeg_encode_close(AVCodecContext *avctx)
> >>
> >> static av_cold int ljpeg_encode_init(AVCodecContext *avctx)
> >> {
> >> + int ret = ff_mjpeg_encode_check_pix_fmt(avctx);
> >> LJpegEncContext *s = avctx->priv_data;
> >>
> >> - if ((avctx->pix_fmt == AV_PIX_FMT_YUV420P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV422P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV444P ||
> >> - avctx->color_range == AVCOL_RANGE_MPEG) &&
> >> - avctx->color_range != AVCOL_RANGE_JPEG &&
> >> - avctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
> >> - av_log(avctx, AV_LOG_ERROR,
> >> - "Non full-range YUV is non-standard, set strict_std_compliance "
> >> - "to at most unofficial to use it.\n");
> >> - return AVERROR(EINVAL);
> >> - }
> >> + if (ret < 0)
> >> + return ret;
> >>
> >> #if FF_API_CODED_FRAME
> >> FF_DISABLE_DEPRECATION_WARNINGS
> >> diff --git a/libavcodec/mjpegenc.c b/libavcodec/mjpegenc.c
> >> index e5d2e24d66..3cff50abdb 100644
> >> --- a/libavcodec/mjpegenc.c
> >> +++ b/libavcodec/mjpegenc.c
> >> @@ -261,9 +261,16 @@ static int alloc_huffman(MpegEncContext *s)
> >> av_cold int ff_mjpeg_encode_init(MpegEncContext *s)
> >> {
> >> MJpegContext *m;
> >> + int ret;
> >>
> >> av_assert0(s->slice_context_count == 1);
> >>
> >> + /* The following check is automatically true for AMV,
> >> + * but it doesn't hurt either. */
> >> + ret = ff_mjpeg_encode_check_pix_fmt(s->avctx);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> if (s->width > 65500 || s->height > 65500) {
> >> av_log(s, AV_LOG_ERROR, "JPEG does not support resolutions above 65500x65500\n");
> >> return AVERROR(EINVAL);
> >> @@ -609,7 +616,9 @@ AVCodec ff_mjpeg_encoder = {
> >> .capabilities = AV_CODEC_CAP_SLICE_THREADS | AV_CODEC_CAP_FRAME_THREADS,
> >> .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP,
> >> .pix_fmts = (const enum AVPixelFormat[]) {
> >> - AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P, AV_PIX_FMT_NONE
> >> + AV_PIX_FMT_YUVJ420P, AV_PIX_FMT_YUVJ422P, AV_PIX_FMT_YUVJ444P,
> >> + AV_PIX_FMT_YUV420P, AV_PIX_FMT_YUV422P, AV_PIX_FMT_YUV444P,
> >> + AV_PIX_FMT_NONE
> >> },
> >> .priv_class = &mjpeg_class,
> >> .profiles = NULL_IF_CONFIG_SMALL(ff_mjpeg_profiles),
> >> diff --git a/libavcodec/mjpegenc_common.c b/libavcodec/mjpegenc_common.c
> >> index 3eae9b7d0f..c1b842d547 100644
> >> --- a/libavcodec/mjpegenc_common.c
> >> +++ b/libavcodec/mjpegenc_common.c
> >> @@ -436,3 +436,19 @@ void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
> >> put_sbits(pb, nbits, mant);
> >> }
> >> }
> >> +
> >> +int ff_mjpeg_encode_check_pix_fmt(AVCodecContext *avctx)
> >> +{
> >> + if (avctx->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL &&
> >> + avctx->color_range != AVCOL_RANGE_JPEG &&
> >> + (avctx->pix_fmt == AV_PIX_FMT_YUV420P ||
> >> + avctx->pix_fmt == AV_PIX_FMT_YUV422P ||
> >> + avctx->pix_fmt == AV_PIX_FMT_YUV444P ||
> >> + avctx->color_range == AVCOL_RANGE_MPEG)) {
> >> + av_log(avctx, AV_LOG_ERROR,
> >> + "Non full-range YUV is non-standard, set strict_std_compliance "
> >> + "to at most unofficial to use it.\n");
> >> + return AVERROR(EINVAL);
> >> + }
> >> + return 0;
> >> +}
> >> diff --git a/libavcodec/mjpegenc_common.h b/libavcodec/mjpegenc_common.h
> >> index b4f8a08e11..76c236d835 100644
> >> --- a/libavcodec/mjpegenc_common.h
> >> +++ b/libavcodec/mjpegenc_common.h
> >> @@ -41,4 +41,6 @@ void ff_mjpeg_init_hvsample(AVCodecContext *avctx, int hsample[4], int vsample[4
> >> void ff_mjpeg_encode_dc(PutBitContext *pb, int val,
> >> uint8_t *huff_size, uint16_t *huff_code);
> >>
> >> +int ff_mjpeg_encode_check_pix_fmt(AVCodecContext *avctx);
> >> +
> >> #endif /* AVCODEC_MJPEGENC_COMMON_H */
> >> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
> >> index 0f38f63de3..d5dd69b88f 100644
> >> --- a/libavcodec/mpegvideo_enc.c
> >> +++ b/libavcodec/mpegvideo_enc.c
> >> @@ -297,36 +297,10 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
> >> {
> >> MpegEncContext *s = avctx->priv_data;
> >> AVCPBProperties *cpb_props;
> >> - int i, ret, format_supported;
> >> + int i, ret;
> >>
> >> mpv_encode_defaults(s);
> >>
> >> - switch (avctx->codec_id) {
> >> - case AV_CODEC_ID_MJPEG:
> >> - format_supported = 0;
> >> - /* JPEG color space */
> >> - if (avctx->pix_fmt == AV_PIX_FMT_YUVJ420P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUVJ422P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUVJ444P ||
> >> - (avctx->color_range == AVCOL_RANGE_JPEG &&
> >> - (avctx->pix_fmt == AV_PIX_FMT_YUV420P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV422P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV444P)))
> >> - format_supported = 1;
> >> - /* MPEG color space */
> >> - else if (avctx->strict_std_compliance <= FF_COMPLIANCE_UNOFFICIAL &&
> >> - (avctx->pix_fmt == AV_PIX_FMT_YUV420P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV422P ||
> >> - avctx->pix_fmt == AV_PIX_FMT_YUV444P))
> >> - format_supported = 1;
> >> -
> >> - if (!format_supported) {
> >> - av_log(avctx, AV_LOG_ERROR, "colorspace not supported in jpeg\n");
> >> - return AVERROR(EINVAL);
> >> - }
> >> - break;
> >> - }
> >> -
> >> switch (avctx->pix_fmt) {
> >> case AV_PIX_FMT_YUVJ444P:
> >> case AV_PIX_FMT_YUV444P:
> >> --
> >> 2.27.0
> >>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
More information about the ffmpeg-devel
mailing list