[FFmpeg-devel] [PATCH 6/9] avcodec/mjpegenc: Include all supported pix_fmts in mpegenc pix_fmts

Andreas Rheinhardt andreas.rheinhardt at outlook.com
Tue Apr 13 20:33:51 EEST 2021


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).

- 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
>>



More information about the ffmpeg-devel mailing list