[FFmpeg-devel] [PATCH] avcodec: add Amuse Graphics decoder

[Paul B Mahol]

On 10/13/18, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Wed, Oct 10, 2018 at 01:22:24PM +0200, Paul B Mahol wrote:
>> This work is partially sponsored by VideoLAN.
>>
>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>> ---
>>
>
>> The AGM3 variant decodes with some artifacts.
>
> I looked a bit as you asked on IRC but wasnt able to really fix it in
> the time i had available
>
> The following patch avoids the green blocks but it does not feel correct
>
> diff --git a/libavcodec/agm.c b/libavcodec/agm.c
> index 424b906eac..a6d9a9a7a6 100644
> --- a/libavcodec/agm.c
> +++ b/libavcodec/agm.c
> @@ -358,10 +358,12 @@ static int decode_inter_plane(AGMContext *s,
> GetBitContext *gb, int size,
>                  if (ret < 0)
>                      return ret;
>
> -                if (mv_x < min) {
> +                if (mv_x < min && s->block[0] > 512) {
>                      s->idsp.idct_put(frame->data[plane] + (s->blocks_h - 1
> - y) * 8 * frame->linesize[plane] + x * 8,
>                                       frame->linesize[plane], s->block);
>                  } else {
> +                    if (mv_x < min)
> +                        mv_x = 0;
>                      if (y * 8 + mv_y < 0 || y * 8 + mv_y >= h ||
>                          x * 8 + mv_x < 0 || x * 8 + mv_x >= w)
>                          return AVERROR_INVALIDDATA;
> [...]
>
>> +
>> +static int read_code2(GetBitContext *gb, int *oskip, int *level)
>> +{
>> +    int max, len = 0, skip = 0;
>> +
>> +    if (show_bits(gb, 2)) {
>> +        switch (show_bits(gb, 4)) {
>> +        case 1:
>> +        case 9:
>> +            len = 1;
>> +            skip = 3;
>> +            break;
>> +        case 2:
>> +            len = 3;
>> +            skip = 4;
>> +            break;
>> +        case 3:
>> +            len = 7;
>> +            skip = 4;
>> +            break;
>> +        case 5:
>> +        case 0xD:
>> +            len = 2;
>> +            skip = 3;
>> +            break;
>> +        case 6:
>> +            len = 4;
>> +            skip = 4;
>> +            break;
>> +        case 7:
>> +            len = 8;
>> +            skip = 4;
>> +            break;
>> +        case 0xA:
>> +            len = 5;
>> +            skip = 4;
>> +            break;
>> +        case 0xB:
>> +            len = 9;
>> +            skip = 4;
>> +            break;
>> +        case 0xE:
>> +            len = 6;
>> +            skip = 4;
>> +            break;
>> +        case 0xF:
>> +            len = ((show_bits(gb, 5) & 0x10) | 0xA0) >> 4;
>> +            skip = 5;
>> +            break;
>> +        default:
>> +            return AVERROR_INVALIDDATA;
>> +        }
>> +        skip_bits(gb, skip);
>> +        *level = get_bits(gb, len);
>> +        *oskip = 0;
>> +        max = 1 << (len - 1);
>> +        if (*level < max)
>> +            *level = -(max + *level);
>> +    } else if (show_bits(gb, 3) & 4) {
>> +        skip_bits(gb, 3);
>
>> +        if (show_bits(gb, 4)) {
>> +            if (show_bits(gb, 4) == 1) {
>> +                skip_bits(gb, 4);
>> +                *oskip = get_bits(gb, 16);
>> +                *level = 0;
>> +            } else {
>> +                *oskip = get_bits(gb, 4);
>> +                *level = 0;
>> +            }
>> +        } else {
>> +            skip_bits(gb, 4);
>> +            *oskip = get_bits(gb, 10);
>> +            *level = 0;
>> +        }
>
> The oskip values which can be represented as shorter codes here (if they
> occur) likely are errors, this could be checked for. Not sure its useful
>
>
> [...]
>
>> +
>> +static int decode_frame(AVCodecContext *avctx, void *data,
>> +                        int *got_frame, AVPacket *avpkt)
>> +{
>> +    AGMContext *s = avctx->priv_data;
>> +    GetBitContext *gb = &s->gb;
>> +    GetByteContext *gbyte = &s->gbyte;
>> +    AVFrame *frame = data;
>> +    int w, h, header;
>> +    int ret;
>> +
>> +    if (!avpkt->size)
>> +        return 0;
>> +
>> +    bytestream2_init(gbyte, avpkt->data, avpkt->size);
>> +
>> +    header = bytestream2_get_le32(gbyte);
>> +    if (header)
>> +        avpriv_request_sample(avctx, "header %X", header);
>> +    s->bitstream_size = bytestream2_get_le24(gbyte);
>> +    if (avpkt->size < s->bitstream_size + 8)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    s->key_frame = bytestream2_get_byte(gbyte) == 32;
>> +    frame->key_frame = s->key_frame;
>> +    frame->pict_type = s->key_frame ? AV_PICTURE_TYPE_I :
>> AV_PICTURE_TYPE_P;
>> +
>> +    s->flags = 0;
>> +    w = bytestream2_get_le32(gbyte);
>> +    if (w < 0) {
>> +        w = -w;
>> +        s->flags |= 2;
>> +    }
>> +    h = bytestream2_get_le32(gbyte);
>> +    if (h < 0) {
>> +        avpriv_request_sample(avctx, "negative height");
>> +        h = -h;
>> +        s->flags |= 1;
>> +    }
>> +
>> +    ret = ff_set_dimensions(avctx, w, h);
>> +    if (ret < 0)
>> +        return ret;
>
> i suspect this has issues with odd width / height
> the motion vectors, and luma and chroma plane sizes probably wont align and
> then
> likely not work as intended as each rounds differently

width/height & 7 is not supported, dimensions are simply extended and
unused/padded
pixels are black.

>
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Old school: Use the lowest level language in which you can solve the
> problem
>             conveniently.
> New school: Use the highest level language in which the latest
> supercomputer
>             can solve the problem without the user falling asleep waiting.
>