[FFmpeg-devel] [PATCH] Document and validate AVFrame plane pointers.

wm4 nfxjfg at googlemail.com
Sat Feb 27 13:38:54 CET 2016


On Sat, 27 Feb 2016 13:01:40 +0100
Reimar Döffinger <Reimar.Doeffinger at gmx.de> wrote:

> Check that the required plane pointers and only
> those are set up.
> Currently does not enforce anything for the palette
> pointer of pseudopal formats as I am unsure about the
> requirements.
> 
> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
> ---
>  doc/APIchanges       |  6 ++++++
>  libavcodec/utils.c   | 27 +++++++++++++++++++++++++++
>  libavcodec/version.h |  2 +-
>  libavutil/frame.h    |  3 +++
>  4 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/APIchanges b/doc/APIchanges
> index 4e952a8..ee407da 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -15,6 +15,12 @@ libavutil:     2015-08-28
>  
>  API changes, most recent first:
>  
> +2016-02-xx - xxxxxxx - lavc 57.25.101
> +  Validate AVFrame returned by get_buffer2 to have required
> +  planes not NULL and unused planes set to NULL as crashes
> +  and buffer overflow are possible with certain streams if
> +  that is not the case.
> +
>  2016-xx-xx - lavc 57.25.0 - avcodec.h
>    Add AVCodecContext.hw_frames_ctx.
>  
> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> index af21cdd..1967f03 100644
> --- a/libavcodec/utils.c
> +++ b/libavcodec/utils.c
> @@ -853,6 +853,31 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame *frame)
>      return ff_init_buffer_info(avctx, frame);
>  }
>  
> +static void validate_avframe_allocation(AVCodecContext *avctx, AVFrame *frame)
> +{
> +    if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
> +        int i;
> +        int num_planes = av_pix_fmt_count_planes(frame->format);
> +        const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(frame->format);
> +        int flags = desc ? desc->flags : 0;
> +        if (num_planes == 1 && (flags & AV_PIX_FMT_FLAG_PAL))
> +            num_planes = 2;
> +        for (i = 0; i < num_planes; i++) {
> +            av_assert0(frame->data[i]);
> +        }
> +        // TODO: what are the exact requirements on pseudopal formats for get_buffer2?
> +        if (num_planes == 1 && (flags & AV_PIX_FMT_FLAG_PSEUDOPAL))
> +            num_planes = 2;

This should be correct. Not allocating a palette for unpaletted but
"pseudopal" formats can cause random crashes, as all code within FFmpeg
seems to assume that the palette has been allocated. (An utter
abomination, if you ask me.) At the same time, pixdesc will indicate
that paletted formats have only 1 plane, but 2 data pointers.

> +        // for formats without data like hwaccel allow
> +        // pointers to be non-NULL.
> +        for (i = num_planes; num_planes > 0 && i < FF_ARRAY_ELEMS(frame->data); i++) {
> +            if (frame->data[i])
> +                av_log(avctx, AV_LOG_ERROR, "Buffer returned by get_buffer2() did not zero unused plane pointers\n");
> +            frame->data[i] = NULL;
> +        }
> +    }
> +}
> +
>  static int get_buffer_internal(AVCodecContext *avctx, AVFrame *frame, int flags)
>  {
>      const AVHWAccel *hwaccel = avctx->hwaccel;
> @@ -885,6 +910,8 @@ static int get_buffer_internal(AVCodecContext *avctx, AVFrame *frame, int flags)
>          avctx->sw_pix_fmt = avctx->pix_fmt;
>  
>      ret = avctx->get_buffer2(avctx, frame, flags);
> +    if (ret >= 0)
> +        validate_avframe_allocation(avctx, frame);
>  
>  end:
>      if (avctx->codec_type == AVMEDIA_TYPE_VIDEO && !override_dimensions) {
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 0856b19..c937eaf 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>  
>  #define LIBAVCODEC_VERSION_MAJOR  57
>  #define LIBAVCODEC_VERSION_MINOR  25
> -#define LIBAVCODEC_VERSION_MICRO 101
> +#define LIBAVCODEC_VERSION_MICRO 102
>  
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 56001a8..76a8123 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -187,6 +187,9 @@ typedef struct AVFrame {
>       * see avcodec_align_dimensions2(). Some filters and swscale can read
>       * up to 16 bytes beyond the planes, if these filters are to be used,
>       * then 16 extra bytes must be allocated.
> +     *
> +     * NOTE: Except for hwaccel formats, pointers not needed by the format
> +     * MUST be set to NULL.
>       */
>      uint8_t *data[AV_NUM_DATA_POINTERS];
>  



More information about the ffmpeg-devel mailing list