[FFmpeg-devel] [PATCH v2 1/3] avcodec/mpegvideo_enc: add checks for custom inter/intra/chroma matrices

[Marton Balint]

On Sat, 11 Jan 2025, Andreas Rheinhardt wrote:

> Marton Balint:
>> Make the checker functions available for all codecs.
>>
>> Signed-off-by: Marton Balint <cus at passwd.hu>
>> ---
>>  libavcodec/encode.c        | 19 +++++++++++++++++++
>>  libavcodec/encode.h        |  9 +++++++++
>>  libavcodec/mpegvideo_enc.c |  8 ++++++++
>>  3 files changed, 36 insertions(+)
>>
>> diff --git a/libavcodec/encode.c b/libavcodec/encode.c
>> index 3baf5b8103..cd10dcf3cd 100644
>> --- a/libavcodec/encode.c
>> +++ b/libavcodec/encode.c
>> @@ -936,3 +936,22 @@ AVCPBProperties *ff_encode_add_cpb_side_data(AVCodecContext *avctx)
>>
>>      return props;
>>  }
>> +
>> +int ff_check_codec_matrices(AVCodecContext *avctx, unsigned types, uint16_t min, uint16_t max)
>> +{
>> +    uint16_t  *matrices[] = {avctx->intra_matrix, avctx->inter_matrix, avctx->chroma_intra_matrix};
>> +    const char   *names[] = {"Intra", "Inter", "Chroma Intra"};
>> +    static_assert(FF_ARRAY_ELEMS(matrices) == FF_ARRAY_ELEMS(names), "matrix count mismatch");
>> +    for (int m = 0; m < FF_ARRAY_ELEMS(matrices); m++) {
>> +        uint16_t *matrix = matrices[m];
>> +        if (matrix && (types & (1U << m))) {
>> +            for (int i = 0; i < 64; i++) {
>> +                if (matrix[i] < min || matrix[i] > max) {
>> +                    av_log(avctx, AV_LOG_ERROR, "%s matrix[%d] is %d which is out of the allowed range [%"PRIu16"-%"PRIu16"].\n", names[m], i, matrix[i], min, max);
>> +                    return AVERROR(EINVAL);
>> +                }
>> +            }
>> +        }
>> +    }
>> +    return 0;
>> +}
>> diff --git a/libavcodec/encode.h b/libavcodec/encode.h
>> index 85331e04b7..656da135d2 100644
>> --- a/libavcodec/encode.h
>> +++ b/libavcodec/encode.h
>> @@ -96,4 +96,13 @@ static av_always_inline int64_t ff_samples_to_time_base(const AVCodecContext *av
>>                          avctx->time_base);
>>  }
>>
>> +/**
>> + * Check if the elements of codec context matrices (intra_matrix, inter_matrix or
>> + * chroma_intra_matrix) are within the specified range.
>> + */
>> +#define FF_MATRIX_TYPE_INTRA        (1U << 0)
>> +#define FF_MATRIX_TYPE_INTER        (1U << 1)
>> +#define FF_MATRIX_TYPE_CHROMA_INTRA (1U << 2)
>> +int ff_check_codec_matrices(AVCodecContext *avctx, unsigned types, uint16_t min, uint16_t max);
>> +
>>  #endif /* AVCODEC_ENCODE_H */
>> diff --git a/libavcodec/mpegvideo_enc.c b/libavcodec/mpegvideo_enc.c
>> index c5f20c2d85..36e7307d23 100644
>> --- a/libavcodec/mpegvideo_enc.c
>> +++ b/libavcodec/mpegvideo_enc.c
>> @@ -987,6 +987,10 @@ av_cold int ff_mpv_encode_init(AVCodecContext *avctx)
>>      /* precompute matrix */
>>      /* for mjpeg, we do include qscale in the matrix */
>>      if (s->out_format != FMT_MJPEG) {
>> +        ret = ff_check_codec_matrices(avctx, FF_MATRIX_TYPE_INTRA | FF_MATRIX_TYPE_INTER, 1, 255);
>> +        if (ret < 0)
>> +            return ret;
>> +
>>          ff_convert_matrix(s, s->q_intra_matrix, s->q_intra_matrix16,
>>                            s->intra_matrix, s->intra_quant_bias, avctx->qmin,
>>                            31, 1);
>> @@ -3754,6 +3758,10 @@ static int encode_picture(MpegEncContext *s, const AVPacket *pkt)
>>          const uint16_t *  luma_matrix = ff_mpeg1_default_intra_matrix;
>>          const uint16_t *chroma_matrix = ff_mpeg1_default_intra_matrix;
>>
>> +        ret = ff_check_codec_matrices(s->avctx, FF_MATRIX_TYPE_INTRA | FF_MATRIX_TYPE_CHROMA_INTRA, (7 + s->qscale) / s->qscale, 65535);
>> +        if (ret < 0)
>> +            return ret;
>> +
>>          if (s->avctx->intra_matrix) {
>>              chroma_matrix =
>>              luma_matrix = s->avctx->intra_matrix;
>
> Is there an intended user besides mpegvideo? If not, it should be in
> mpegvideo_enc.c until there is another user.

As far as I see libxvid makes use of avctx->intra/inter_matrix, so a check 
should be added there as well.

Regards,
Marton