[FFmpeg-devel] avcodec: add Amuse Graphics decoder

Submitted by Michael Niedermayer on Oct. 13, 2018, 2:34 p.m.

Details

Message ID 20181013143452.GJ26838@michaelspb
State New
Headers show

Commit Message

Michael Niedermayer Oct. 13, 2018, 2:34 p.m.
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@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

[...]

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


[...]

Comments

Paul B Mahol Oct. 13, 2018, 3:19 p.m.
On 10/13/18, Michael Niedermayer <michael@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@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.
>

Patch hide | download patch | download mbox

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;