[libav-stable] vmd: refactor the inner decode loop

Reinhard Tartler siretart at gmail.com
Fri May 31 22:54:40 CEST 2013


Hi Luca,

this patch does not apply cleanly to release/9, and I'd feel much more
comfortable, if you could backport and test it.

Thanks,
Reinhard

On Wed, May 29, 2013 at 12:12 PM, Luca Barbato <git at libav.org> wrote:
> Module: libav
> Branch: master
> Commit: 676da248cad49debc40720baa13214f0b94dcc71
>
> Author:    Luca Barbato <lu_zero at gentoo.org>
> Committer: Luca Barbato <lu_zero at gentoo.org>
> Date:      Tue May 28 22:09:59 2013 +0200
>
> vmd: refactor the inner decode loop
>
> Simplify a little, assume empty frames are acceptable and
> do not pointlessly reinit the bytestream2 contexts using
> possibly wrong size values.
>
> Reported-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> CC: libav-stable at libav.org
>
> ---
>
>  libavcodec/vmdav.c |  158 ++++++++++++++++++++++++++-------------------------
>  1 files changed, 81 insertions(+), 77 deletions(-)
>
> diff --git a/libavcodec/vmdav.c b/libavcodec/vmdav.c
> index 5e31f55..d7f43f0 100644
> --- a/libavcodec/vmdav.c
> +++ b/libavcodec/vmdav.c
> @@ -268,91 +268,95 @@ static int vmd_decode(VmdVideoContext *s, AVFrame *frame)
>          }
>          s->size -= PALETTE_COUNT * 3 + 2;
>      }
> -    if (s->size > 0) {
> -        /* originally UnpackFrame in VAG's code */
> -        bytestream2_init(&gb, gb.buffer, s->buf + s->size - gb.buffer);
> -        if (bytestream2_get_bytes_left(&gb) < 1)
> -            return AVERROR_INVALIDDATA;
> -        meth = bytestream2_get_byteu(&gb);
> -        if (meth & 0x80) {
> -            lz_unpack(gb.buffer, bytestream2_get_bytes_left(&gb),
> -                      s->unpack_buffer, s->unpack_buffer_size);
> -            meth &= 0x7F;
> -            bytestream2_init(&gb, s->unpack_buffer, s->unpack_buffer_size);
> -        }
>
> -        dp = &frame->data[0][frame_y * frame->linesize[0] + frame_x];
> -        pp = &s->prev_frame.data[0][frame_y * s->prev_frame.linesize[0] + frame_x];
> -        switch (meth) {
> -        case 1:
> -            for (i = 0; i < frame_height; i++) {
> -                ofs = 0;
> -                do {
> -                    len = bytestream2_get_byte(&gb);
> -                    if (len & 0x80) {
> -                        len = (len & 0x7F) + 1;
> -                        if (ofs + len > frame_width || bytestream2_get_bytes_left(&gb) < len)
> -                            return AVERROR_INVALIDDATA;
> -                        bytestream2_get_buffer(&gb, &dp[ofs], len);
> -                        ofs += len;
> -                    } else {
> -                        /* interframe pixel copy */
> -                        if (ofs + len + 1 > frame_width || !s->prev_frame.data[0])
> -                            return AVERROR_INVALIDDATA;
> -                        memcpy(&dp[ofs], &pp[ofs], len + 1);
> -                        ofs += len + 1;
> -                    }
> -                } while (ofs < frame_width);
> -                if (ofs > frame_width) {
> -                    av_log(s->avctx, AV_LOG_ERROR, "VMD video: offset > width (%d > %d)\n",
> -                        ofs, frame_width);
> -                    return AVERROR_INVALIDDATA;
> -                }
> -                dp += frame->linesize[0];
> -                pp += s->prev_frame.linesize[0];
> -            }
> -            break;
> +    if (!s->size)
> +        return 0;
> +
> +    /* originally UnpackFrame in VAG's code */
> +    if (bytestream2_get_bytes_left(&gb) < 1)
> +        return AVERROR_INVALIDDATA;
> +    meth = bytestream2_get_byteu(&gb);
> +    if (meth & 0x80) {
> +        lz_unpack(gb.buffer, bytestream2_get_bytes_left(&gb),
> +                  s->unpack_buffer, s->unpack_buffer_size);
> +        meth &= 0x7F;
> +        bytestream2_init(&gb, s->unpack_buffer, s->unpack_buffer_size);
> +    }
>
> -        case 2:
> -            for (i = 0; i < frame_height; i++) {
> -                bytestream2_get_buffer(&gb, dp, frame_width);
> -                dp += frame->linesize[0];
> -                pp += s->prev_frame.linesize[0];
> +    dp = &frame->data[0][frame_y * frame->linesize[0] + frame_x];
> +    pp = &s->prev_frame.data[0][frame_y * s->prev_frame.linesize[0] + frame_x];
> +    switch (meth) {
> +    case 1:
> +        for (i = 0; i < frame_height; i++) {
> +            ofs = 0;
> +            do {
> +                len = bytestream2_get_byte(&gb);
> +                if (len & 0x80) {
> +                    len = (len & 0x7F) + 1;
> +                    if (ofs + len > frame_width ||
> +                        bytestream2_get_bytes_left(&gb) < len)
> +                        return AVERROR_INVALIDDATA;
> +                    bytestream2_get_buffer(&gb, &dp[ofs], len);
> +                    ofs += len;
> +                } else {
> +                    /* interframe pixel copy */
> +                    if (ofs + len + 1 > frame_width || !s->prev_frame.data[0])
> +                        return AVERROR_INVALIDDATA;
> +                    memcpy(&dp[ofs], &pp[ofs], len + 1);
> +                    ofs += len + 1;
> +                }
> +            } while (ofs < frame_width);
> +            if (ofs > frame_width) {
> +                av_log(s->avctx, AV_LOG_ERROR,
> +                       "VMD video: offset > width (%d > %d)\n",
> +                       ofs, frame_width);
> +                return AVERROR_INVALIDDATA;
>              }
> -            break;
> +            dp += frame->linesize[0];
> +            pp += s->prev_frame.linesize[0];
> +        }
> +        break;
>
> -        case 3:
> -            for (i = 0; i < frame_height; i++) {
> -                ofs = 0;
> -                do {
> -                    len = bytestream2_get_byte(&gb);
> -                    if (len & 0x80) {
> -                        len = (len & 0x7F) + 1;
> -                        if (bytestream2_get_byte(&gb) == 0xFF)
> -                            len = rle_unpack(gb.buffer, &dp[ofs],
> -                                             len, bytestream2_get_bytes_left(&gb),
> -                                             frame_width - ofs);
> -                        else
> -                            bytestream2_get_buffer(&gb, &dp[ofs], len);
> -                        bytestream2_skip(&gb, len);
> -                    } else {
> -                        /* interframe pixel copy */
> -                        if (ofs + len + 1 > frame_width || !s->prev_frame.data[0])
> -                            return AVERROR_INVALIDDATA;
> -                        memcpy(&dp[ofs], &pp[ofs], len + 1);
> -                        ofs += len + 1;
> -                    }
> -                } while (ofs < frame_width);
> -                if (ofs > frame_width) {
> -                    av_log(s->avctx, AV_LOG_ERROR, "VMD video: offset > width (%d > %d)\n",
> -                        ofs, frame_width);
> -                    return AVERROR_INVALIDDATA;
> +    case 2:
> +        for (i = 0; i < frame_height; i++) {
> +            bytestream2_get_buffer(&gb, dp, frame_width);
> +            dp += frame->linesize[0];
> +            pp += s->prev_frame.linesize[0];
> +        }
> +        break;
> +
> +    case 3:
> +        for (i = 0; i < frame_height; i++) {
> +            ofs = 0;
> +            do {
> +                len = bytestream2_get_byte(&gb);
> +                if (len & 0x80) {
> +                    len = (len & 0x7F) + 1;
> +                    if (bytestream2_get_byte(&gb) == 0xFF)
> +                        len = rle_unpack(gb.buffer, &dp[ofs],
> +                                         len, bytestream2_get_bytes_left(&gb),
> +                                         frame_width - ofs);
> +                    else
> +                        bytestream2_get_buffer(&gb, &dp[ofs], len);
> +                    bytestream2_skip(&gb, len);
> +                } else {
> +                    /* interframe pixel copy */
> +                    if (ofs + len + 1 > frame_width || !s->prev_frame.data[0])
> +                        return AVERROR_INVALIDDATA;
> +                    memcpy(&dp[ofs], &pp[ofs], len + 1);
> +                    ofs += len + 1;
>                  }
> -                dp += frame->linesize[0];
> -                pp += s->prev_frame.linesize[0];
> +            } while (ofs < frame_width);
> +            if (ofs > frame_width) {
> +                av_log(s->avctx, AV_LOG_ERROR,
> +                       "VMD video: offset > width (%d > %d)\n",
> +                       ofs, frame_width);
> +                return AVERROR_INVALIDDATA;
>              }
> -            break;
> +            dp += frame->linesize[0];
> +            pp += s->prev_frame.linesize[0];
>          }
> +        break;
>      }
>      return 0;
>  }
>
> _______________________________________________
> libav-stable mailing list
> libav-stable at libav.org
> https://lists.libav.org/mailman/listinfo/libav-stable



-- 
regards,
    Reinhard


More information about the libav-stable mailing list