diff mbox

[FFmpeg-devel] avcodec: add ARBC decoder

Message ID 20190122095533.14932-1-onemda@gmail.com
State Accepted
Headers show

Commit Message

Paul B Mahol Jan. 22, 2019, 9:55 a.m. UTC
Thanks Kostya for great help in reversing binary.

Signed-off-by: Paul B Mahol <onemda@gmail.com>
---
 libavcodec/Makefile     |   1 +
 libavcodec/allcodecs.c  |   1 +
 libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
 libavcodec/avcodec.h    |   1 +
 libavcodec/codec_desc.c |   7 ++
 libavformat/riff.c      |   1 +
 6 files changed, 214 insertions(+)
 create mode 100644 libavcodec/arbc.c

Comments

Moritz Barsnick Jan. 22, 2019, 1:18 p.m. UTC | #1
On Tue, Jan 22, 2019 at 10:55:33 +0100, Paul B Mahol wrote:
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/avcodec.h    |   1 +
>  libavcodec/codec_desc.c |   7 ++
>  libavformat/riff.c      |   1 +

A changelog entry and a doc entry are welcomed. :-)

Cheers,
Moritz
James Almer Jan. 22, 2019, 1:22 p.m. UTC | #2
On 1/22/2019 6:55 AM, Paul B Mahol wrote:
> Thanks Kostya for great help in reversing binary.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/avcodec.h    |   1 +
>  libavcodec/codec_desc.c |   7 ++
>  libavformat/riff.c      |   1 +
>  6 files changed, 214 insertions(+)
>  create mode 100644 libavcodec/arbc.c
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index bf746c143d..8e240aecf0 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index fe0376e27e..5cbb09a5a4 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>  extern AVCodec ff_ansi_decoder;
>  extern AVCodec ff_apng_encoder;
>  extern AVCodec ff_apng_decoder;
> +extern AVCodec ff_arbc_decoder;
>  extern AVCodec ff_asv1_encoder;
>  extern AVCodec ff_asv1_decoder;
>  extern AVCodec ff_asv2_encoder;
> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
> new file mode 100644
> index 0000000000..59a1d7bf0a
> --- /dev/null
> +++ b/libavcodec/arbc.c
> @@ -0,0 +1,203 @@
> +/*
> + * Gryphon's Anim Compressor decoder
> + * Copyright (c) 2018 Paul B Mahol
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "libavutil/imgutils.h"
> +#include "libavutil/internal.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/mem.h"
> +
> +#include "avcodec.h"
> +#include "bytestream.h"
> +#include "internal.h"
> +
> +typedef struct ARBCContext {
> +    GetByteContext gb;
> +
> +    AVFrame *prev_frame;
> +} ARBCContext;
> +
> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame *frame)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +    GetByteContext *gb = &s->gb;
> +    int nb_tiles = bytestream2_get_le16(gb);
> +    int h = avctx->height - 1;
> +
> +    for (int i = 0; i < nb_tiles; i++) {
> +        int y = bytestream2_get_byte(gb);
> +        int x = bytestream2_get_byte(gb);
> +        uint16_t mask = bytestream2_get_le16(gb);
> +        int start_y = y * 4, start_x = x * 4;
> +        int end_y = start_y + 4, end_x = start_x + 4;
> +
> +        for (int j = start_y; j < end_y; j++) {
> +            for (int k = start_x; k < end_x; k++) {
> +                if (mask & 0x8000) {
> +                    if (j >= avctx->height || k >= avctx->width)
> +                        continue;
> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 0] = color[0];
> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 1] = color[1];
> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 2] = color[2];
> +                }
> +                mask = mask << 1;

get_bits(). Same below.

> +            }
> +        }
> +    }
> +}
> +
> +static void fill_tileX(AVCodecContext *avctx, int tile_width, int tile_height,
> +                       uint8_t *color, AVFrame *frame)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +    GetByteContext *gb = &s->gb;
> +    const int step_h = tile_height / 4;
> +    const int step_w = tile_width / 4;
> +    int nb_tiles = bytestream2_get_le16(gb);
> +    int h = avctx->height - 1;
> +
> +    for (int i = 0; i < nb_tiles; i++) {
> +        int y = bytestream2_get_byte(gb);
> +        int x = bytestream2_get_byte(gb);
> +        uint16_t mask = bytestream2_get_le16(gb);
> +        int start_y = y * tile_height, start_x = x * tile_width;
> +        int end_y = start_y + tile_height, end_x = start_x + tile_width;
> +
> +        for (int j = start_y; j < end_y; j += step_h) {
> +            for (int k = start_x; k < end_x; k += step_w) {
> +                if (mask & 0x8000U) {
> +                    for (int m = 0; m < step_h; m++) {
> +                        for (int n = 0; n < step_w; n++) {
> +                            if (j + m >= avctx->height || k + n >= avctx->width)
> +                                continue;
> +                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 0] = color[0];
> +                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 1] = color[1];
> +                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 2] = color[2];
> +                        }
> +                    }
> +                }
> +                mask = mask << 1;
> +            }
> +        }
> +    }
> +}
> +
> +static int decode_frame(AVCodecContext *avctx, void *data,
> +                        int *got_frame, AVPacket *avpkt)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +    AVFrame *const frame = data;

No const.

> +    int ret, nb_segments, keyframe = 1;
> +
> +    if (avpkt->size < 1)
> +        return AVERROR_INVALIDDATA;
> +
> +    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
> +        return ret;
> +
> +    if (s->prev_frame->data[0]) {
> +        ret = av_frame_copy(frame, s->prev_frame);
> +        if (ret < 0)
> +            return ret;
> +    }
> +
> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
> +    bytestream2_skip(&s->gb, 8);
> +    nb_segments = bytestream2_get_le16(&s->gb);
> +    if (nb_segments == 0)
> +        keyframe = 0;
> +
> +    for (int i = 0; i < nb_segments; i++) {
> +        int resolution_flag;
> +        uint8_t fill[3];
> +
> +        if (bytestream2_get_bytes_left(&s->gb) <= 0)
> +            return AVERROR_INVALIDDATA;
> +
> +        fill[0] = bytestream2_get_byte(&s->gb);
> +        bytestream2_skip(&s->gb, 1);
> +        fill[1] = bytestream2_get_byte(&s->gb);
> +        bytestream2_skip(&s->gb, 1);
> +        fill[2] = bytestream2_get_byte(&s->gb);
> +        bytestream2_skip(&s->gb, 1);
> +        resolution_flag = bytestream2_get_byte(&s->gb);
> +
> +        if (resolution_flag & 0x10)
> +            fill_tileX(avctx, 1024, 1024, fill, frame);
> +        if (resolution_flag & 0x08)
> +            fill_tileX(avctx, 256, 256, fill, frame);
> +        if (resolution_flag & 0x04)
> +            fill_tileX(avctx, 64, 64, fill, frame);
> +        if (resolution_flag & 0x02)
> +            fill_tileX(avctx, 16, 16, fill, frame);
> +        if (resolution_flag & 0x01)
> +            fill_tile4(avctx, fill, frame);
> +    }
> +
> +    av_frame_unref(s->prev_frame);
> +    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
> +        return ret;
> +
> +    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
> +    frame->key_frame = keyframe;
> +    *got_frame = 1;
> +
> +    return avpkt->size;
> +}
> +
> +static av_cold int decode_init(AVCodecContext *avctx)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +
> +    avctx->pix_fmt = AV_PIX_FMT_RGB24;
> +
> +    s->prev_frame = av_frame_alloc();
> +    if (!s->prev_frame)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
> +}
> +
> +static av_cold int decode_close(AVCodecContext *avctx)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +
> +    av_frame_free(&s->prev_frame);
> +
> +    return 0;
> +}
> +
> +AVCodec ff_arbc_decoder = {
> +    .name           = "arbc",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
> +    .type           = AVMEDIA_TYPE_VIDEO,
> +    .id             = AV_CODEC_ID_ARBC,
> +    .priv_data_size = sizeof(ARBCContext),
> +    .init           = decode_init,
> +    .decode         = decode_frame,
> +    .close          = decode_close,
> +    .capabilities   = AV_CODEC_CAP_DR1,
> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
> +                      FF_CODEC_CAP_INIT_CLEANUP,

No need for this one. Only s->prev_frame is allocated in init(), and if
that fails then close() would do nothing.

What's missing is AV_CODEC_CAP_DR1 seeing you use get_buffer.

> +};
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 90f9f08289..72dc277dfd 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -453,6 +453,7 @@ enum AVCodecID {
>      AV_CODEC_ID_WCMV,
>      AV_CODEC_ID_RASC,
>      AV_CODEC_ID_HYMT,
> +    AV_CODEC_ID_ARBC,
>  
>      /* various PCM "codecs" */
>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at the start of audio codecs
> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
> index 10a639101c..7b254c1d45 100644
> --- a/libavcodec/codec_desc.c
> +++ b/libavcodec/codec_desc.c
> @@ -1698,6 +1698,13 @@ static const AVCodecDescriptor codec_descriptors[] = {
>          .long_name = NULL_IF_CONFIG_SMALL("HuffYUV MT"),
>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>      },
> +    {
> +        .id        = AV_CODEC_ID_ARBC,
> +        .type      = AVMEDIA_TYPE_VIDEO,
> +        .name      = "arbc",
> +        .long_name = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
> +        .props     = AV_CODEC_PROP_LOSSY,
> +    },
>  
>      /* various PCM "codecs" */
>      {
> diff --git a/libavformat/riff.c b/libavformat/riff.c
> index d5a509c8b9..8f0fd99e22 100644
> --- a/libavformat/riff.c
> +++ b/libavformat/riff.c
> @@ -476,6 +476,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
>      { AV_CODEC_ID_WCMV,         MKTAG('W', 'C', 'M', 'V') },
>      { AV_CODEC_ID_RASC,         MKTAG('R', 'A', 'S', 'C') },
>      { AV_CODEC_ID_HYMT,         MKTAG('H', 'Y', 'M', 'T') },
> +    { AV_CODEC_ID_ARBC,         MKTAG('A', 'R', 'B', 'C') },
>      { AV_CODEC_ID_NONE,         0 }
>  };
>  
>
Paul B Mahol Jan. 22, 2019, 1:30 p.m. UTC | #3
On 1/22/19, James Almer <jamrial@gmail.com> wrote:
> On 1/22/2019 6:55 AM, Paul B Mahol wrote:
>> Thanks Kostya for great help in reversing binary.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/Makefile     |   1 +
>>  libavcodec/allcodecs.c  |   1 +
>>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>>  libavcodec/avcodec.h    |   1 +
>>  libavcodec/codec_desc.c |   7 ++
>>  libavformat/riff.c      |   1 +
>>  6 files changed, 214 insertions(+)
>>  create mode 100644 libavcodec/arbc.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index bf746c143d..8e240aecf0 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index fe0376e27e..5cbb09a5a4 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>>  extern AVCodec ff_ansi_decoder;
>>  extern AVCodec ff_apng_encoder;
>>  extern AVCodec ff_apng_decoder;
>> +extern AVCodec ff_arbc_decoder;
>>  extern AVCodec ff_asv1_encoder;
>>  extern AVCodec ff_asv1_decoder;
>>  extern AVCodec ff_asv2_encoder;
>> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
>> new file mode 100644
>> index 0000000000..59a1d7bf0a
>> --- /dev/null
>> +++ b/libavcodec/arbc.c
>> @@ -0,0 +1,203 @@
>> +/*
>> + * Gryphon's Anim Compressor decoder
>> + * Copyright (c) 2018 Paul B Mahol
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +
>> +#include "libavutil/imgutils.h"
>> +#include "libavutil/internal.h"
>> +#include "libavutil/intreadwrite.h"
>> +#include "libavutil/mem.h"
>> +
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +#include "internal.h"
>> +
>> +typedef struct ARBCContext {
>> +    GetByteContext gb;
>> +
>> +    AVFrame *prev_frame;
>> +} ARBCContext;
>> +
>> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame
>> *frame)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +    GetByteContext *gb = &s->gb;
>> +    int nb_tiles = bytestream2_get_le16(gb);
>> +    int h = avctx->height - 1;
>> +
>> +    for (int i = 0; i < nb_tiles; i++) {
>> +        int y = bytestream2_get_byte(gb);
>> +        int x = bytestream2_get_byte(gb);
>> +        uint16_t mask = bytestream2_get_le16(gb);
>> +        int start_y = y * 4, start_x = x * 4;
>> +        int end_y = start_y + 4, end_x = start_x + 4;
>> +
>> +        for (int j = start_y; j < end_y; j++) {
>> +            for (int k = start_x; k < end_x; k++) {
>> +                if (mask & 0x8000) {
>> +                    if (j >= avctx->height || k >= avctx->width)
>> +                        continue;
>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>> 0] = color[0];
>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>> 1] = color[1];
>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>> 2] = color[2];
>> +                }
>> +                mask = mask << 1;
>
> get_bits(). Same below.

Oh, come one, I had enough of this nonsense.
This is 16 bit number, using fucking get bits is overkill.
Get over it.

>
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +static void fill_tileX(AVCodecContext *avctx, int tile_width, int
>> tile_height,
>> +                       uint8_t *color, AVFrame *frame)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +    GetByteContext *gb = &s->gb;
>> +    const int step_h = tile_height / 4;
>> +    const int step_w = tile_width / 4;
>> +    int nb_tiles = bytestream2_get_le16(gb);
>> +    int h = avctx->height - 1;
>> +
>> +    for (int i = 0; i < nb_tiles; i++) {
>> +        int y = bytestream2_get_byte(gb);
>> +        int x = bytestream2_get_byte(gb);
>> +        uint16_t mask = bytestream2_get_le16(gb);
>> +        int start_y = y * tile_height, start_x = x * tile_width;
>> +        int end_y = start_y + tile_height, end_x = start_x + tile_width;
>> +
>> +        for (int j = start_y; j < end_y; j += step_h) {
>> +            for (int k = start_x; k < end_x; k += step_w) {
>> +                if (mask & 0x8000U) {
>> +                    for (int m = 0; m < step_h; m++) {
>> +                        for (int n = 0; n < step_w; n++) {
>> +                            if (j + m >= avctx->height || k + n >=
>> avctx->width)
>> +                                continue;
>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>> m)) + 3 * (k + n) + 0] = color[0];
>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>> m)) + 3 * (k + n) + 1] = color[1];
>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>> m)) + 3 * (k + n) + 2] = color[2];
>> +                        }
>> +                    }
>> +                }
>> +                mask = mask << 1;
>> +            }
>> +        }
>> +    }
>> +}
>> +
>> +static int decode_frame(AVCodecContext *avctx, void *data,
>> +                        int *got_frame, AVPacket *avpkt)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +    AVFrame *const frame = data;
>
> No const.

Will remove.

>
>> +    int ret, nb_segments, keyframe = 1;
>> +
>> +    if (avpkt->size < 1)
>> +        return AVERROR_INVALIDDATA;
>> +
>> +    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
>> +        return ret;
>> +
>> +    if (s->prev_frame->data[0]) {
>> +        ret = av_frame_copy(frame, s->prev_frame);
>> +        if (ret < 0)
>> +            return ret;
>> +    }
>> +
>> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
>> +    bytestream2_skip(&s->gb, 8);
>> +    nb_segments = bytestream2_get_le16(&s->gb);
>> +    if (nb_segments == 0)
>> +        keyframe = 0;
>> +
>> +    for (int i = 0; i < nb_segments; i++) {
>> +        int resolution_flag;
>> +        uint8_t fill[3];
>> +
>> +        if (bytestream2_get_bytes_left(&s->gb) <= 0)
>> +            return AVERROR_INVALIDDATA;
>> +
>> +        fill[0] = bytestream2_get_byte(&s->gb);
>> +        bytestream2_skip(&s->gb, 1);
>> +        fill[1] = bytestream2_get_byte(&s->gb);
>> +        bytestream2_skip(&s->gb, 1);
>> +        fill[2] = bytestream2_get_byte(&s->gb);
>> +        bytestream2_skip(&s->gb, 1);
>> +        resolution_flag = bytestream2_get_byte(&s->gb);
>> +
>> +        if (resolution_flag & 0x10)
>> +            fill_tileX(avctx, 1024, 1024, fill, frame);
>> +        if (resolution_flag & 0x08)
>> +            fill_tileX(avctx, 256, 256, fill, frame);
>> +        if (resolution_flag & 0x04)
>> +            fill_tileX(avctx, 64, 64, fill, frame);
>> +        if (resolution_flag & 0x02)
>> +            fill_tileX(avctx, 16, 16, fill, frame);
>> +        if (resolution_flag & 0x01)
>> +            fill_tile4(avctx, fill, frame);
>> +    }
>> +
>> +    av_frame_unref(s->prev_frame);
>> +    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
>> +        return ret;
>> +
>> +    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
>> +    frame->key_frame = keyframe;
>> +    *got_frame = 1;
>> +
>> +    return avpkt->size;
>> +}
>> +
>> +static av_cold int decode_init(AVCodecContext *avctx)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +
>> +    avctx->pix_fmt = AV_PIX_FMT_RGB24;
>> +
>> +    s->prev_frame = av_frame_alloc();
>> +    if (!s->prev_frame)
>> +        return AVERROR(ENOMEM);
>> +
>> +    return 0;
>> +}
>> +
>> +static av_cold int decode_close(AVCodecContext *avctx)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +
>> +    av_frame_free(&s->prev_frame);
>> +
>> +    return 0;
>> +}
>> +
>> +AVCodec ff_arbc_decoder = {
>> +    .name           = "arbc",
>> +    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
>> +    .type           = AVMEDIA_TYPE_VIDEO,
>> +    .id             = AV_CODEC_ID_ARBC,
>> +    .priv_data_size = sizeof(ARBCContext),
>> +    .init           = decode_init,
>> +    .decode         = decode_frame,
>> +    .close          = decode_close,
>> +    .capabilities   = AV_CODEC_CAP_DR1,
>> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
>> +                      FF_CODEC_CAP_INIT_CLEANUP,
>
> No need for this one. Only s->prev_frame is allocated in init(), and if
> that fails then close() would do nothing.

What? Nonsense.

>
> What's missing is AV_CODEC_CAP_DR1 seeing you use get_buffer.

Sorry, but you are not making any sense here. DR1 is right above.

>
>> +};
>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> index 90f9f08289..72dc277dfd 100644
>> --- a/libavcodec/avcodec.h
>> +++ b/libavcodec/avcodec.h
>> @@ -453,6 +453,7 @@ enum AVCodecID {
>>      AV_CODEC_ID_WCMV,
>>      AV_CODEC_ID_RASC,
>>      AV_CODEC_ID_HYMT,
>> +    AV_CODEC_ID_ARBC,
>>
>>      /* various PCM "codecs" */
>>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at
>> the start of audio codecs
>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>> index 10a639101c..7b254c1d45 100644
>> --- a/libavcodec/codec_desc.c
>> +++ b/libavcodec/codec_desc.c
>> @@ -1698,6 +1698,13 @@ static const AVCodecDescriptor codec_descriptors[]
>> = {
>>          .long_name = NULL_IF_CONFIG_SMALL("HuffYUV MT"),
>>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>>      },
>> +    {
>> +        .id        = AV_CODEC_ID_ARBC,
>> +        .type      = AVMEDIA_TYPE_VIDEO,
>> +        .name      = "arbc",
>> +        .long_name = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
>> +        .props     = AV_CODEC_PROP_LOSSY,
>> +    },
>>
>>      /* various PCM "codecs" */
>>      {
>> diff --git a/libavformat/riff.c b/libavformat/riff.c
>> index d5a509c8b9..8f0fd99e22 100644
>> --- a/libavformat/riff.c
>> +++ b/libavformat/riff.c
>> @@ -476,6 +476,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
>>      { AV_CODEC_ID_WCMV,         MKTAG('W', 'C', 'M', 'V') },
>>      { AV_CODEC_ID_RASC,         MKTAG('R', 'A', 'S', 'C') },
>>      { AV_CODEC_ID_HYMT,         MKTAG('H', 'Y', 'M', 'T') },
>> +    { AV_CODEC_ID_ARBC,         MKTAG('A', 'R', 'B', 'C') },
>>      { AV_CODEC_ID_NONE,         0 }
>>  };
>>
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
James Almer Jan. 22, 2019, 2:03 p.m. UTC | #4
On 1/22/2019 10:30 AM, Paul B Mahol wrote:
> On 1/22/19, James Almer <jamrial@gmail.com> wrote:
>> On 1/22/2019 6:55 AM, Paul B Mahol wrote:
>>> Thanks Kostya for great help in reversing binary.
>>>
>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>> ---
>>>  libavcodec/Makefile     |   1 +
>>>  libavcodec/allcodecs.c  |   1 +
>>>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>>>  libavcodec/avcodec.h    |   1 +
>>>  libavcodec/codec_desc.c |   7 ++
>>>  libavformat/riff.c      |   1 +
>>>  6 files changed, 214 insertions(+)
>>>  create mode 100644 libavcodec/arbc.c
>>>
>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>> index bf746c143d..8e240aecf0 100644
>>> --- a/libavcodec/Makefile
>>> +++ b/libavcodec/Makefile
>>> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>>>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>>>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>>>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>>> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>>>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>>>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>>>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
>>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>>> index fe0376e27e..5cbb09a5a4 100644
>>> --- a/libavcodec/allcodecs.c
>>> +++ b/libavcodec/allcodecs.c
>>> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>>>  extern AVCodec ff_ansi_decoder;
>>>  extern AVCodec ff_apng_encoder;
>>>  extern AVCodec ff_apng_decoder;
>>> +extern AVCodec ff_arbc_decoder;
>>>  extern AVCodec ff_asv1_encoder;
>>>  extern AVCodec ff_asv1_decoder;
>>>  extern AVCodec ff_asv2_encoder;
>>> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
>>> new file mode 100644
>>> index 0000000000..59a1d7bf0a
>>> --- /dev/null
>>> +++ b/libavcodec/arbc.c
>>> @@ -0,0 +1,203 @@
>>> +/*
>>> + * Gryphon's Anim Compressor decoder
>>> + * Copyright (c) 2018 Paul B Mahol
>>> + *
>>> + * This file is part of FFmpeg.
>>> + *
>>> + * FFmpeg is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU Lesser General Public
>>> + * License as published by the Free Software Foundation; either
>>> + * version 2.1 of the License, or (at your option) any later version.
>>> + *
>>> + * FFmpeg is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + * Lesser General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU Lesser General Public
>>> + * License along with FFmpeg; if not, write to the Free Software
>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>> 02110-1301 USA
>>> + */
>>> +
>>> +#include <stdio.h>
>>> +#include <stdlib.h>
>>> +#include <string.h>
>>> +
>>> +#include "libavutil/imgutils.h"
>>> +#include "libavutil/internal.h"
>>> +#include "libavutil/intreadwrite.h"
>>> +#include "libavutil/mem.h"
>>> +
>>> +#include "avcodec.h"
>>> +#include "bytestream.h"
>>> +#include "internal.h"
>>> +
>>> +typedef struct ARBCContext {
>>> +    GetByteContext gb;
>>> +
>>> +    AVFrame *prev_frame;
>>> +} ARBCContext;
>>> +
>>> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame
>>> *frame)
>>> +{
>>> +    ARBCContext *s = avctx->priv_data;
>>> +    GetByteContext *gb = &s->gb;
>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>> +    int h = avctx->height - 1;
>>> +
>>> +    for (int i = 0; i < nb_tiles; i++) {
>>> +        int y = bytestream2_get_byte(gb);
>>> +        int x = bytestream2_get_byte(gb);
>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>> +        int start_y = y * 4, start_x = x * 4;
>>> +        int end_y = start_y + 4, end_x = start_x + 4;
>>> +
>>> +        for (int j = start_y; j < end_y; j++) {
>>> +            for (int k = start_x; k < end_x; k++) {
>>> +                if (mask & 0x8000) {
>>> +                    if (j >= avctx->height || k >= avctx->width)
>>> +                        continue;
>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>>> 0] = color[0];
>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>>> 1] = color[1];
>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k +
>>> 2] = color[2];
>>> +                }
>>> +                mask = mask << 1;
>>
>> get_bits(). Same below.
> 
> Oh, come one, I had enough of this nonsense.
> This is 16 bit number, using fucking get bits is overkill.
> Get over it.

Don't get pissy about nothing. I asked nothing that deserves that kind
of reaction.

Doing mask & 0x8000 then shifting it left by one at every loop is weird
and one of the reasons why get_bits was written for. But do however you
prefer.

> 
>>
>>> +            }
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static void fill_tileX(AVCodecContext *avctx, int tile_width, int
>>> tile_height,
>>> +                       uint8_t *color, AVFrame *frame)
>>> +{
>>> +    ARBCContext *s = avctx->priv_data;
>>> +    GetByteContext *gb = &s->gb;
>>> +    const int step_h = tile_height / 4;
>>> +    const int step_w = tile_width / 4;
>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>> +    int h = avctx->height - 1;
>>> +
>>> +    for (int i = 0; i < nb_tiles; i++) {
>>> +        int y = bytestream2_get_byte(gb);
>>> +        int x = bytestream2_get_byte(gb);
>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>> +        int start_y = y * tile_height, start_x = x * tile_width;
>>> +        int end_y = start_y + tile_height, end_x = start_x + tile_width;
>>> +
>>> +        for (int j = start_y; j < end_y; j += step_h) {
>>> +            for (int k = start_x; k < end_x; k += step_w) {
>>> +                if (mask & 0x8000U) {
>>> +                    for (int m = 0; m < step_h; m++) {
>>> +                        for (int n = 0; n < step_w; n++) {
>>> +                            if (j + m >= avctx->height || k + n >=
>>> avctx->width)
>>> +                                continue;
>>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>>> m)) + 3 * (k + n) + 0] = color[0];
>>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>>> m)) + 3 * (k + n) + 1] = color[1];
>>> +                            frame->data[0][frame->linesize[0] * (h - (j +
>>> m)) + 3 * (k + n) + 2] = color[2];
>>> +                        }
>>> +                    }
>>> +                }
>>> +                mask = mask << 1;
>>> +            }
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static int decode_frame(AVCodecContext *avctx, void *data,
>>> +                        int *got_frame, AVPacket *avpkt)
>>> +{
>>> +    ARBCContext *s = avctx->priv_data;
>>> +    AVFrame *const frame = data;
>>
>> No const.
> 
> Will remove.
> 
>>
>>> +    int ret, nb_segments, keyframe = 1;
>>> +
>>> +    if (avpkt->size < 1)
>>> +        return AVERROR_INVALIDDATA;
>>> +
>>> +    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
>>> +        return ret;
>>> +
>>> +    if (s->prev_frame->data[0]) {
>>> +        ret = av_frame_copy(frame, s->prev_frame);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +    }
>>> +
>>> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
>>> +    bytestream2_skip(&s->gb, 8);
>>> +    nb_segments = bytestream2_get_le16(&s->gb);
>>> +    if (nb_segments == 0)
>>> +        keyframe = 0;
>>> +
>>> +    for (int i = 0; i < nb_segments; i++) {
>>> +        int resolution_flag;
>>> +        uint8_t fill[3];
>>> +
>>> +        if (bytestream2_get_bytes_left(&s->gb) <= 0)
>>> +            return AVERROR_INVALIDDATA;
>>> +
>>> +        fill[0] = bytestream2_get_byte(&s->gb);
>>> +        bytestream2_skip(&s->gb, 1);
>>> +        fill[1] = bytestream2_get_byte(&s->gb);
>>> +        bytestream2_skip(&s->gb, 1);
>>> +        fill[2] = bytestream2_get_byte(&s->gb);
>>> +        bytestream2_skip(&s->gb, 1);
>>> +        resolution_flag = bytestream2_get_byte(&s->gb);
>>> +
>>> +        if (resolution_flag & 0x10)
>>> +            fill_tileX(avctx, 1024, 1024, fill, frame);
>>> +        if (resolution_flag & 0x08)
>>> +            fill_tileX(avctx, 256, 256, fill, frame);
>>> +        if (resolution_flag & 0x04)
>>> +            fill_tileX(avctx, 64, 64, fill, frame);
>>> +        if (resolution_flag & 0x02)
>>> +            fill_tileX(avctx, 16, 16, fill, frame);
>>> +        if (resolution_flag & 0x01)
>>> +            fill_tile4(avctx, fill, frame);
>>> +    }
>>> +
>>> +    av_frame_unref(s->prev_frame);
>>> +    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
>>> +        return ret;
>>> +
>>> +    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
>>> +    frame->key_frame = keyframe;
>>> +    *got_frame = 1;
>>> +
>>> +    return avpkt->size;
>>> +}
>>> +
>>> +static av_cold int decode_init(AVCodecContext *avctx)
>>> +{
>>> +    ARBCContext *s = avctx->priv_data;
>>> +
>>> +    avctx->pix_fmt = AV_PIX_FMT_RGB24;
>>> +
>>> +    s->prev_frame = av_frame_alloc();
>>> +    if (!s->prev_frame)
>>> +        return AVERROR(ENOMEM);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +static av_cold int decode_close(AVCodecContext *avctx)
>>> +{
>>> +    ARBCContext *s = avctx->priv_data;
>>> +
>>> +    av_frame_free(&s->prev_frame);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +AVCodec ff_arbc_decoder = {
>>> +    .name           = "arbc",
>>> +    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
>>> +    .type           = AVMEDIA_TYPE_VIDEO,
>>> +    .id             = AV_CODEC_ID_ARBC,
>>> +    .priv_data_size = sizeof(ARBCContext),
>>> +    .init           = decode_init,
>>> +    .decode         = decode_frame,
>>> +    .close          = decode_close,
>>> +    .capabilities   = AV_CODEC_CAP_DR1,
>>> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
>>> +                      FF_CODEC_CAP_INIT_CLEANUP,
>>
>> No need for this one. Only s->prev_frame is allocated in init(), and if
>> that fails then close() would do nothing.
> 
> What? Nonsense.

INIT_CLEANUP is a flag to tell avcodec_open2() to call AVCodec.close()
if AVCodec.init() fails. Since s->prev_frame is the only thing allocated
by init(), calling close() after that failed to allocate is pointless.
s->prev_frame will be NULL and close() will be essentially a nop.

> 
>>
>> What's missing is AV_CODEC_CAP_DR1 seeing you use get_buffer.
> 
> Sorry, but you are not making any sense here. DR1 is right above.

Right, missed it.

> 
>>
>>> +};
>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>> index 90f9f08289..72dc277dfd 100644
>>> --- a/libavcodec/avcodec.h
>>> +++ b/libavcodec/avcodec.h
>>> @@ -453,6 +453,7 @@ enum AVCodecID {
>>>      AV_CODEC_ID_WCMV,
>>>      AV_CODEC_ID_RASC,
>>>      AV_CODEC_ID_HYMT,
>>> +    AV_CODEC_ID_ARBC,
>>>
>>>      /* various PCM "codecs" */
>>>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at
>>> the start of audio codecs
>>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>>> index 10a639101c..7b254c1d45 100644
>>> --- a/libavcodec/codec_desc.c
>>> +++ b/libavcodec/codec_desc.c
>>> @@ -1698,6 +1698,13 @@ static const AVCodecDescriptor codec_descriptors[]
>>> = {
>>>          .long_name = NULL_IF_CONFIG_SMALL("HuffYUV MT"),
>>>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>>>      },
>>> +    {
>>> +        .id        = AV_CODEC_ID_ARBC,
>>> +        .type      = AVMEDIA_TYPE_VIDEO,
>>> +        .name      = "arbc",
>>> +        .long_name = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
>>> +        .props     = AV_CODEC_PROP_LOSSY,
>>> +    },
>>>
>>>      /* various PCM "codecs" */
>>>      {
>>> diff --git a/libavformat/riff.c b/libavformat/riff.c
>>> index d5a509c8b9..8f0fd99e22 100644
>>> --- a/libavformat/riff.c
>>> +++ b/libavformat/riff.c
>>> @@ -476,6 +476,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
>>>      { AV_CODEC_ID_WCMV,         MKTAG('W', 'C', 'M', 'V') },
>>>      { AV_CODEC_ID_RASC,         MKTAG('R', 'A', 'S', 'C') },
>>>      { AV_CODEC_ID_HYMT,         MKTAG('H', 'Y', 'M', 'T') },
>>> +    { AV_CODEC_ID_ARBC,         MKTAG('A', 'R', 'B', 'C') },
>>>      { AV_CODEC_ID_NONE,         0 }
>>>  };
>>>
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Jean-Baptiste Kempf Jan. 22, 2019, 2:21 p.m. UTC | #5
On Tue, 22 Jan 2019, at 10:55, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda@gmail.com>

Overall, this is more readable that the ones in the past. Really cool.

> +        for (int j = start_y; j < end_y; j++) {
> +            for (int k = start_x; k < end_x; k++) {
> +                if (mask & 0x8000) {

Do we know where this value comes from?
Paul B Mahol Jan. 22, 2019, 2:54 p.m. UTC | #6
On 1/22/19, James Almer <jamrial@gmail.com> wrote:
> On 1/22/2019 10:30 AM, Paul B Mahol wrote:
>> On 1/22/19, James Almer <jamrial@gmail.com> wrote:
>>> On 1/22/2019 6:55 AM, Paul B Mahol wrote:
>>>> Thanks Kostya for great help in reversing binary.
>>>>
>>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>>> ---
>>>>  libavcodec/Makefile     |   1 +
>>>>  libavcodec/allcodecs.c  |   1 +
>>>>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>>>>  libavcodec/avcodec.h    |   1 +
>>>>  libavcodec/codec_desc.c |   7 ++
>>>>  libavformat/riff.c      |   1 +
>>>>  6 files changed, 214 insertions(+)
>>>>  create mode 100644 libavcodec/arbc.c
>>>>
>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>> index bf746c143d..8e240aecf0 100644
>>>> --- a/libavcodec/Makefile
>>>> +++ b/libavcodec/Makefile
>>>> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>>>>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>>>>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>>>>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>>>> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>>>>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>>>>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>>>>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
>>>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>>>> index fe0376e27e..5cbb09a5a4 100644
>>>> --- a/libavcodec/allcodecs.c
>>>> +++ b/libavcodec/allcodecs.c
>>>> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>>>>  extern AVCodec ff_ansi_decoder;
>>>>  extern AVCodec ff_apng_encoder;
>>>>  extern AVCodec ff_apng_decoder;
>>>> +extern AVCodec ff_arbc_decoder;
>>>>  extern AVCodec ff_asv1_encoder;
>>>>  extern AVCodec ff_asv1_decoder;
>>>>  extern AVCodec ff_asv2_encoder;
>>>> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
>>>> new file mode 100644
>>>> index 0000000000..59a1d7bf0a
>>>> --- /dev/null
>>>> +++ b/libavcodec/arbc.c
>>>> @@ -0,0 +1,203 @@
>>>> +/*
>>>> + * Gryphon's Anim Compressor decoder
>>>> + * Copyright (c) 2018 Paul B Mahol
>>>> + *
>>>> + * This file is part of FFmpeg.
>>>> + *
>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU Lesser General Public
>>>> + * License as published by the Free Software Foundation; either
>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>> + *
>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>> + * Lesser General Public License for more details.
>>>> + *
>>>> + * You should have received a copy of the GNU Lesser General Public
>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>> 02110-1301 USA
>>>> + */
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <stdlib.h>
>>>> +#include <string.h>
>>>> +
>>>> +#include "libavutil/imgutils.h"
>>>> +#include "libavutil/internal.h"
>>>> +#include "libavutil/intreadwrite.h"
>>>> +#include "libavutil/mem.h"
>>>> +
>>>> +#include "avcodec.h"
>>>> +#include "bytestream.h"
>>>> +#include "internal.h"
>>>> +
>>>> +typedef struct ARBCContext {
>>>> +    GetByteContext gb;
>>>> +
>>>> +    AVFrame *prev_frame;
>>>> +} ARBCContext;
>>>> +
>>>> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame
>>>> *frame)
>>>> +{
>>>> +    ARBCContext *s = avctx->priv_data;
>>>> +    GetByteContext *gb = &s->gb;
>>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>>> +    int h = avctx->height - 1;
>>>> +
>>>> +    for (int i = 0; i < nb_tiles; i++) {
>>>> +        int y = bytestream2_get_byte(gb);
>>>> +        int x = bytestream2_get_byte(gb);
>>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>>> +        int start_y = y * 4, start_x = x * 4;
>>>> +        int end_y = start_y + 4, end_x = start_x + 4;
>>>> +
>>>> +        for (int j = start_y; j < end_y; j++) {
>>>> +            for (int k = start_x; k < end_x; k++) {
>>>> +                if (mask & 0x8000) {
>>>> +                    if (j >= avctx->height || k >= avctx->width)
>>>> +                        continue;
>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>> +
>>>> 0] = color[0];
>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>> +
>>>> 1] = color[1];
>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>> +
>>>> 2] = color[2];
>>>> +                }
>>>> +                mask = mask << 1;
>>>
>>> get_bits(). Same below.
>>
>> Oh, come one, I had enough of this nonsense.
>> This is 16 bit number, using fucking get bits is overkill.
>> Get over it.
>
> Don't get pissy about nothing. I asked nothing that deserves that kind
> of reaction.
>
> Doing mask & 0x8000 then shifting it left by one at every loop is weird
> and one of the reasons why get_bits was written for. But do however you
> prefer.

Correct me if I'm wrong, but I can not use get_bits here for all data
(something its read little endian sometimes it picks bits from top),
and using it only for
single 16bit number is silly.

>
>>
>>>
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>> +static void fill_tileX(AVCodecContext *avctx, int tile_width, int
>>>> tile_height,
>>>> +                       uint8_t *color, AVFrame *frame)
>>>> +{
>>>> +    ARBCContext *s = avctx->priv_data;
>>>> +    GetByteContext *gb = &s->gb;
>>>> +    const int step_h = tile_height / 4;
>>>> +    const int step_w = tile_width / 4;
>>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>>> +    int h = avctx->height - 1;
>>>> +
>>>> +    for (int i = 0; i < nb_tiles; i++) {
>>>> +        int y = bytestream2_get_byte(gb);
>>>> +        int x = bytestream2_get_byte(gb);
>>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>>> +        int start_y = y * tile_height, start_x = x * tile_width;
>>>> +        int end_y = start_y + tile_height, end_x = start_x +
>>>> tile_width;
>>>> +
>>>> +        for (int j = start_y; j < end_y; j += step_h) {
>>>> +            for (int k = start_x; k < end_x; k += step_w) {
>>>> +                if (mask & 0x8000U) {
>>>> +                    for (int m = 0; m < step_h; m++) {
>>>> +                        for (int n = 0; n < step_w; n++) {
>>>> +                            if (j + m >= avctx->height || k + n >=
>>>> avctx->width)
>>>> +                                continue;
>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>> +
>>>> m)) + 3 * (k + n) + 0] = color[0];
>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>> +
>>>> m)) + 3 * (k + n) + 1] = color[1];
>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>> +
>>>> m)) + 3 * (k + n) + 2] = color[2];
>>>> +                        }
>>>> +                    }
>>>> +                }
>>>> +                mask = mask << 1;
>>>> +            }
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>> +static int decode_frame(AVCodecContext *avctx, void *data,
>>>> +                        int *got_frame, AVPacket *avpkt)
>>>> +{
>>>> +    ARBCContext *s = avctx->priv_data;
>>>> +    AVFrame *const frame = data;
>>>
>>> No const.
>>
>> Will remove.
>>
>>>
>>>> +    int ret, nb_segments, keyframe = 1;
>>>> +
>>>> +    if (avpkt->size < 1)
>>>> +        return AVERROR_INVALIDDATA;
>>>> +
>>>> +    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) <
>>>> 0)
>>>> +        return ret;
>>>> +
>>>> +    if (s->prev_frame->data[0]) {
>>>> +        ret = av_frame_copy(frame, s->prev_frame);
>>>> +        if (ret < 0)
>>>> +            return ret;
>>>> +    }
>>>> +
>>>> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
>>>> +    bytestream2_skip(&s->gb, 8);
>>>> +    nb_segments = bytestream2_get_le16(&s->gb);
>>>> +    if (nb_segments == 0)
>>>> +        keyframe = 0;
>>>> +
>>>> +    for (int i = 0; i < nb_segments; i++) {
>>>> +        int resolution_flag;
>>>> +        uint8_t fill[3];
>>>> +
>>>> +        if (bytestream2_get_bytes_left(&s->gb) <= 0)
>>>> +            return AVERROR_INVALIDDATA;
>>>> +
>>>> +        fill[0] = bytestream2_get_byte(&s->gb);
>>>> +        bytestream2_skip(&s->gb, 1);
>>>> +        fill[1] = bytestream2_get_byte(&s->gb);
>>>> +        bytestream2_skip(&s->gb, 1);
>>>> +        fill[2] = bytestream2_get_byte(&s->gb);
>>>> +        bytestream2_skip(&s->gb, 1);
>>>> +        resolution_flag = bytestream2_get_byte(&s->gb);
>>>> +
>>>> +        if (resolution_flag & 0x10)
>>>> +            fill_tileX(avctx, 1024, 1024, fill, frame);
>>>> +        if (resolution_flag & 0x08)
>>>> +            fill_tileX(avctx, 256, 256, fill, frame);
>>>> +        if (resolution_flag & 0x04)
>>>> +            fill_tileX(avctx, 64, 64, fill, frame);
>>>> +        if (resolution_flag & 0x02)
>>>> +            fill_tileX(avctx, 16, 16, fill, frame);
>>>> +        if (resolution_flag & 0x01)
>>>> +            fill_tile4(avctx, fill, frame);
>>>> +    }
>>>> +
>>>> +    av_frame_unref(s->prev_frame);
>>>> +    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
>>>> +        return ret;
>>>> +
>>>> +    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I :
>>>> AV_PICTURE_TYPE_P;
>>>> +    frame->key_frame = keyframe;
>>>> +    *got_frame = 1;
>>>> +
>>>> +    return avpkt->size;
>>>> +}
>>>> +
>>>> +static av_cold int decode_init(AVCodecContext *avctx)
>>>> +{
>>>> +    ARBCContext *s = avctx->priv_data;
>>>> +
>>>> +    avctx->pix_fmt = AV_PIX_FMT_RGB24;
>>>> +
>>>> +    s->prev_frame = av_frame_alloc();
>>>> +    if (!s->prev_frame)
>>>> +        return AVERROR(ENOMEM);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static av_cold int decode_close(AVCodecContext *avctx)
>>>> +{
>>>> +    ARBCContext *s = avctx->priv_data;
>>>> +
>>>> +    av_frame_free(&s->prev_frame);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +AVCodec ff_arbc_decoder = {
>>>> +    .name           = "arbc",
>>>> +    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim
>>>> Compressor"),
>>>> +    .type           = AVMEDIA_TYPE_VIDEO,
>>>> +    .id             = AV_CODEC_ID_ARBC,
>>>> +    .priv_data_size = sizeof(ARBCContext),
>>>> +    .init           = decode_init,
>>>> +    .decode         = decode_frame,
>>>> +    .close          = decode_close,
>>>> +    .capabilities   = AV_CODEC_CAP_DR1,
>>>> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
>>>> +                      FF_CODEC_CAP_INIT_CLEANUP,
>>>
>>> No need for this one. Only s->prev_frame is allocated in init(), and if
>>> that fails then close() would do nothing.
>>
>> What? Nonsense.
>
> INIT_CLEANUP is a flag to tell avcodec_open2() to call AVCodec.close()
> if AVCodec.init() fails. Since s->prev_frame is the only thing allocated
> by init(), calling close() after that failed to allocate is pointless.
> s->prev_frame will be NULL and close() will be essentially a nop.

So you do not like this flag being present here? It does not hurt to
have it though.

>
>>
>>>
>>> What's missing is AV_CODEC_CAP_DR1 seeing you use get_buffer.
>>
>> Sorry, but you are not making any sense here. DR1 is right above.
>
> Right, missed it.
>
>>
>>>
>>>> +};
>>>> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>>>> index 90f9f08289..72dc277dfd 100644
>>>> --- a/libavcodec/avcodec.h
>>>> +++ b/libavcodec/avcodec.h
>>>> @@ -453,6 +453,7 @@ enum AVCodecID {
>>>>      AV_CODEC_ID_WCMV,
>>>>      AV_CODEC_ID_RASC,
>>>>      AV_CODEC_ID_HYMT,
>>>> +    AV_CODEC_ID_ARBC,
>>>>
>>>>      /* various PCM "codecs" */
>>>>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at
>>>> the start of audio codecs
>>>> diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
>>>> index 10a639101c..7b254c1d45 100644
>>>> --- a/libavcodec/codec_desc.c
>>>> +++ b/libavcodec/codec_desc.c
>>>> @@ -1698,6 +1698,13 @@ static const AVCodecDescriptor
>>>> codec_descriptors[]
>>>> = {
>>>>          .long_name = NULL_IF_CONFIG_SMALL("HuffYUV MT"),
>>>>          .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
>>>>      },
>>>> +    {
>>>> +        .id        = AV_CODEC_ID_ARBC,
>>>> +        .type      = AVMEDIA_TYPE_VIDEO,
>>>> +        .name      = "arbc",
>>>> +        .long_name = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
>>>> +        .props     = AV_CODEC_PROP_LOSSY,
>>>> +    },
>>>>
>>>>      /* various PCM "codecs" */
>>>>      {
>>>> diff --git a/libavformat/riff.c b/libavformat/riff.c
>>>> index d5a509c8b9..8f0fd99e22 100644
>>>> --- a/libavformat/riff.c
>>>> +++ b/libavformat/riff.c
>>>> @@ -476,6 +476,7 @@ const AVCodecTag ff_codec_bmp_tags[] = {
>>>>      { AV_CODEC_ID_WCMV,         MKTAG('W', 'C', 'M', 'V') },
>>>>      { AV_CODEC_ID_RASC,         MKTAG('R', 'A', 'S', 'C') },
>>>>      { AV_CODEC_ID_HYMT,         MKTAG('H', 'Y', 'M', 'T') },
>>>> +    { AV_CODEC_ID_ARBC,         MKTAG('A', 'R', 'B', 'C') },
>>>>      { AV_CODEC_ID_NONE,         0 }
>>>>  };
>>>>
>>>>
>>>
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Paul B Mahol Jan. 22, 2019, 2:57 p.m. UTC | #7
On 1/22/19, Jean-Baptiste Kempf <jb@videolan.org> wrote:
> On Tue, 22 Jan 2019, at 10:55, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>
> Overall, this is more readable that the ones in the past. Really cool.

With that terms: "more readable" actually means "less complex"

>
>> +        for (int j = start_y; j < end_y; j++) {
>> +            for (int k = start_x; k < end_x; k++) {
>> +                if (mask & 0x8000) {
>
> Do we know where this value comes from?
>

Can't you see? It is obvious. It reads bits from top to bottom of 16
bit number and than does something - filling blocks.
Peter Ross Jan. 22, 2019, 8:48 p.m. UTC | #8
On Tue, Jan 22, 2019 at 10:22:29AM -0300, James Almer wrote:
> On 1/22/2019 6:55 AM, Paul B Mahol wrote:
> > Thanks Kostya for great help in reversing binary.

:)

> > +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame *frame)
> > +{
> > +    ARBCContext *s = avctx->priv_data;
> > +    GetByteContext *gb = &s->gb;
> > +    int nb_tiles = bytestream2_get_le16(gb);
> > +    int h = avctx->height - 1;
> > +
> > +    for (int i = 0; i < nb_tiles; i++) {
> > +        int y = bytestream2_get_byte(gb);
> > +        int x = bytestream2_get_byte(gb);
> > +        uint16_t mask = bytestream2_get_le16(gb);
> > +        int start_y = y * 4, start_x = x * 4;
> > +        int end_y = start_y + 4, end_x = start_x + 4;
> > +
> > +        for (int j = start_y; j < end_y; j++) {
> > +            for (int k = start_x; k < end_x; k++) {
> > +                if (mask & 0x8000) {
> > +                    if (j >= avctx->height || k >= avctx->width)
> > +                        continue;
> > +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 0] = color[0];
> > +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 1] = color[1];
> > +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 2] = color[2];
> > +                }
> > +                mask = mask << 1;
> 
> get_bits(). Same below.

i do not agree, for the overkill reason given by paul.

equally, the pixel blit code could be moved to a seperate inlined function.
this is just a code style suggestion from me.

otherwise looks good.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
James Almer Jan. 22, 2019, 9:12 p.m. UTC | #9
On 1/22/2019 11:54 AM, Paul B Mahol wrote:
> On 1/22/19, James Almer <jamrial@gmail.com> wrote:
>> On 1/22/2019 10:30 AM, Paul B Mahol wrote:
>>> On 1/22/19, James Almer <jamrial@gmail.com> wrote:
>>>> On 1/22/2019 6:55 AM, Paul B Mahol wrote:
>>>>> Thanks Kostya for great help in reversing binary.
>>>>>
>>>>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>>>>> ---
>>>>>  libavcodec/Makefile     |   1 +
>>>>>  libavcodec/allcodecs.c  |   1 +
>>>>>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>>>>>  libavcodec/avcodec.h    |   1 +
>>>>>  libavcodec/codec_desc.c |   7 ++
>>>>>  libavformat/riff.c      |   1 +
>>>>>  6 files changed, 214 insertions(+)
>>>>>  create mode 100644 libavcodec/arbc.c
>>>>>
>>>>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>>>>> index bf746c143d..8e240aecf0 100644
>>>>> --- a/libavcodec/Makefile
>>>>> +++ b/libavcodec/Makefile
>>>>> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>>>>>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>>>>>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>>>>>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>>>>> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>>>>>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>>>>>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>>>>>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
>>>>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>>>>> index fe0376e27e..5cbb09a5a4 100644
>>>>> --- a/libavcodec/allcodecs.c
>>>>> +++ b/libavcodec/allcodecs.c
>>>>> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>>>>>  extern AVCodec ff_ansi_decoder;
>>>>>  extern AVCodec ff_apng_encoder;
>>>>>  extern AVCodec ff_apng_decoder;
>>>>> +extern AVCodec ff_arbc_decoder;
>>>>>  extern AVCodec ff_asv1_encoder;
>>>>>  extern AVCodec ff_asv1_decoder;
>>>>>  extern AVCodec ff_asv2_encoder;
>>>>> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
>>>>> new file mode 100644
>>>>> index 0000000000..59a1d7bf0a
>>>>> --- /dev/null
>>>>> +++ b/libavcodec/arbc.c
>>>>> @@ -0,0 +1,203 @@
>>>>> +/*
>>>>> + * Gryphon's Anim Compressor decoder
>>>>> + * Copyright (c) 2018 Paul B Mahol
>>>>> + *
>>>>> + * This file is part of FFmpeg.
>>>>> + *
>>>>> + * FFmpeg is free software; you can redistribute it and/or
>>>>> + * modify it under the terms of the GNU Lesser General Public
>>>>> + * License as published by the Free Software Foundation; either
>>>>> + * version 2.1 of the License, or (at your option) any later version.
>>>>> + *
>>>>> + * FFmpeg is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>>>> + * Lesser General Public License for more details.
>>>>> + *
>>>>> + * You should have received a copy of the GNU Lesser General Public
>>>>> + * License along with FFmpeg; if not, write to the Free Software
>>>>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>>> 02110-1301 USA
>>>>> + */
>>>>> +
>>>>> +#include <stdio.h>
>>>>> +#include <stdlib.h>
>>>>> +#include <string.h>
>>>>> +
>>>>> +#include "libavutil/imgutils.h"
>>>>> +#include "libavutil/internal.h"
>>>>> +#include "libavutil/intreadwrite.h"
>>>>> +#include "libavutil/mem.h"
>>>>> +
>>>>> +#include "avcodec.h"
>>>>> +#include "bytestream.h"
>>>>> +#include "internal.h"
>>>>> +
>>>>> +typedef struct ARBCContext {
>>>>> +    GetByteContext gb;
>>>>> +
>>>>> +    AVFrame *prev_frame;
>>>>> +} ARBCContext;
>>>>> +
>>>>> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame
>>>>> *frame)
>>>>> +{
>>>>> +    ARBCContext *s = avctx->priv_data;
>>>>> +    GetByteContext *gb = &s->gb;
>>>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>>>> +    int h = avctx->height - 1;
>>>>> +
>>>>> +    for (int i = 0; i < nb_tiles; i++) {
>>>>> +        int y = bytestream2_get_byte(gb);
>>>>> +        int x = bytestream2_get_byte(gb);
>>>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>>>> +        int start_y = y * 4, start_x = x * 4;
>>>>> +        int end_y = start_y + 4, end_x = start_x + 4;
>>>>> +
>>>>> +        for (int j = start_y; j < end_y; j++) {
>>>>> +            for (int k = start_x; k < end_x; k++) {
>>>>> +                if (mask & 0x8000) {
>>>>> +                    if (j >= avctx->height || k >= avctx->width)
>>>>> +                        continue;
>>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>>> +
>>>>> 0] = color[0];
>>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>>> +
>>>>> 1] = color[1];
>>>>> +                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k
>>>>> +
>>>>> 2] = color[2];
>>>>> +                }
>>>>> +                mask = mask << 1;
>>>>
>>>> get_bits(). Same below.
>>>
>>> Oh, come one, I had enough of this nonsense.
>>> This is 16 bit number, using fucking get bits is overkill.
>>> Get over it.
>>
>> Don't get pissy about nothing. I asked nothing that deserves that kind
>> of reaction.
>>
>> Doing mask & 0x8000 then shifting it left by one at every loop is weird
>> and one of the reasons why get_bits was written for. But do however you
>> prefer.
> 
> Correct me if I'm wrong, but I can not use get_bits here for all data
> (something its read little endian sometimes it picks bits from top),

You're right, nb_tiles and nb_segments and this loop probably can't use
the same get_bits context.

> and using it only for
> single 16bit number is silly.
> 
>>
>>>
>>>>
>>>>> +            }
>>>>> +        }
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static void fill_tileX(AVCodecContext *avctx, int tile_width, int
>>>>> tile_height,
>>>>> +                       uint8_t *color, AVFrame *frame)
>>>>> +{
>>>>> +    ARBCContext *s = avctx->priv_data;
>>>>> +    GetByteContext *gb = &s->gb;
>>>>> +    const int step_h = tile_height / 4;
>>>>> +    const int step_w = tile_width / 4;
>>>>> +    int nb_tiles = bytestream2_get_le16(gb);
>>>>> +    int h = avctx->height - 1;
>>>>> +
>>>>> +    for (int i = 0; i < nb_tiles; i++) {
>>>>> +        int y = bytestream2_get_byte(gb);
>>>>> +        int x = bytestream2_get_byte(gb);
>>>>> +        uint16_t mask = bytestream2_get_le16(gb);
>>>>> +        int start_y = y * tile_height, start_x = x * tile_width;
>>>>> +        int end_y = start_y + tile_height, end_x = start_x +
>>>>> tile_width;
>>>>> +
>>>>> +        for (int j = start_y; j < end_y; j += step_h) {
>>>>> +            for (int k = start_x; k < end_x; k += step_w) {
>>>>> +                if (mask & 0x8000U) {
>>>>> +                    for (int m = 0; m < step_h; m++) {
>>>>> +                        for (int n = 0; n < step_w; n++) {
>>>>> +                            if (j + m >= avctx->height || k + n >=
>>>>> avctx->width)
>>>>> +                                continue;
>>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>>> +
>>>>> m)) + 3 * (k + n) + 0] = color[0];
>>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>>> +
>>>>> m)) + 3 * (k + n) + 1] = color[1];
>>>>> +                            frame->data[0][frame->linesize[0] * (h - (j
>>>>> +
>>>>> m)) + 3 * (k + n) + 2] = color[2];
>>>>> +                        }
>>>>> +                    }
>>>>> +                }
>>>>> +                mask = mask << 1;
>>>>> +            }
>>>>> +        }
>>>>> +    }
>>>>> +}
>>>>> +
>>>>> +static int decode_frame(AVCodecContext *avctx, void *data,
>>>>> +                        int *got_frame, AVPacket *avpkt)
>>>>> +{
>>>>> +    ARBCContext *s = avctx->priv_data;
>>>>> +    AVFrame *const frame = data;
>>>>
>>>> No const.
>>>
>>> Will remove.
>>>
>>>>
>>>>> +    int ret, nb_segments, keyframe = 1;
>>>>> +
>>>>> +    if (avpkt->size < 1)
>>>>> +        return AVERROR_INVALIDDATA;
>>>>> +
>>>>> +    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) <
>>>>> 0)
>>>>> +        return ret;
>>>>> +
>>>>> +    if (s->prev_frame->data[0]) {
>>>>> +        ret = av_frame_copy(frame, s->prev_frame);
>>>>> +        if (ret < 0)
>>>>> +            return ret;
>>>>> +    }
>>>>> +
>>>>> +    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
>>>>> +    bytestream2_skip(&s->gb, 8);
>>>>> +    nb_segments = bytestream2_get_le16(&s->gb);
>>>>> +    if (nb_segments == 0)
>>>>> +        keyframe = 0;
>>>>> +
>>>>> +    for (int i = 0; i < nb_segments; i++) {
>>>>> +        int resolution_flag;
>>>>> +        uint8_t fill[3];
>>>>> +
>>>>> +        if (bytestream2_get_bytes_left(&s->gb) <= 0)
>>>>> +            return AVERROR_INVALIDDATA;
>>>>> +
>>>>> +        fill[0] = bytestream2_get_byte(&s->gb);
>>>>> +        bytestream2_skip(&s->gb, 1);
>>>>> +        fill[1] = bytestream2_get_byte(&s->gb);
>>>>> +        bytestream2_skip(&s->gb, 1);
>>>>> +        fill[2] = bytestream2_get_byte(&s->gb);
>>>>> +        bytestream2_skip(&s->gb, 1);
>>>>> +        resolution_flag = bytestream2_get_byte(&s->gb);
>>>>> +
>>>>> +        if (resolution_flag & 0x10)
>>>>> +            fill_tileX(avctx, 1024, 1024, fill, frame);
>>>>> +        if (resolution_flag & 0x08)
>>>>> +            fill_tileX(avctx, 256, 256, fill, frame);
>>>>> +        if (resolution_flag & 0x04)
>>>>> +            fill_tileX(avctx, 64, 64, fill, frame);
>>>>> +        if (resolution_flag & 0x02)
>>>>> +            fill_tileX(avctx, 16, 16, fill, frame);
>>>>> +        if (resolution_flag & 0x01)
>>>>> +            fill_tile4(avctx, fill, frame);
>>>>> +    }
>>>>> +
>>>>> +    av_frame_unref(s->prev_frame);
>>>>> +    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
>>>>> +        return ret;
>>>>> +
>>>>> +    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I :
>>>>> AV_PICTURE_TYPE_P;
>>>>> +    frame->key_frame = keyframe;
>>>>> +    *got_frame = 1;
>>>>> +
>>>>> +    return avpkt->size;
>>>>> +}
>>>>> +
>>>>> +static av_cold int decode_init(AVCodecContext *avctx)
>>>>> +{
>>>>> +    ARBCContext *s = avctx->priv_data;
>>>>> +
>>>>> +    avctx->pix_fmt = AV_PIX_FMT_RGB24;
>>>>> +
>>>>> +    s->prev_frame = av_frame_alloc();
>>>>> +    if (!s->prev_frame)
>>>>> +        return AVERROR(ENOMEM);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +static av_cold int decode_close(AVCodecContext *avctx)
>>>>> +{
>>>>> +    ARBCContext *s = avctx->priv_data;
>>>>> +
>>>>> +    av_frame_free(&s->prev_frame);
>>>>> +
>>>>> +    return 0;
>>>>> +}
>>>>> +
>>>>> +AVCodec ff_arbc_decoder = {
>>>>> +    .name           = "arbc",
>>>>> +    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim
>>>>> Compressor"),
>>>>> +    .type           = AVMEDIA_TYPE_VIDEO,
>>>>> +    .id             = AV_CODEC_ID_ARBC,
>>>>> +    .priv_data_size = sizeof(ARBCContext),
>>>>> +    .init           = decode_init,
>>>>> +    .decode         = decode_frame,
>>>>> +    .close          = decode_close,
>>>>> +    .capabilities   = AV_CODEC_CAP_DR1,
>>>>> +    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
>>>>> +                      FF_CODEC_CAP_INIT_CLEANUP,
>>>>
>>>> No need for this one. Only s->prev_frame is allocated in init(), and if
>>>> that fails then close() would do nothing.
>>>
>>> What? Nonsense.
>>
>> INIT_CLEANUP is a flag to tell avcodec_open2() to call AVCodec.close()
>> if AVCodec.init() fails. Since s->prev_frame is the only thing allocated
>> by init(), calling close() after that failed to allocate is pointless.
>> s->prev_frame will be NULL and close() will be essentially a nop.
> 
> So you do not like this flag being present here? It does not hurt to
> have it though.

It doesn't hurt, but it's also not proper. If someone were to use this
code as a reference implementation of the flag (grepping the tree for
examples), they'd get the wrong idea.
Michael Niedermayer Jan. 22, 2019, 9:16 p.m. UTC | #10
On Tue, Jan 22, 2019 at 10:55:33AM +0100, Paul B Mahol wrote:
> Thanks Kostya for great help in reversing binary.
> 
> Signed-off-by: Paul B Mahol <onemda@gmail.com>
> ---
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>  libavcodec/avcodec.h    |   1 +
>  libavcodec/codec_desc.c |   7 ++
>  libavformat/riff.c      |   1 +
>  6 files changed, 214 insertions(+)
>  create mode 100644 libavcodec/arbc.c
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index bf746c143d..8e240aecf0 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index fe0376e27e..5cbb09a5a4 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>  extern AVCodec ff_ansi_decoder;
>  extern AVCodec ff_apng_encoder;
>  extern AVCodec ff_apng_decoder;
> +extern AVCodec ff_arbc_decoder;
>  extern AVCodec ff_asv1_encoder;
>  extern AVCodec ff_asv1_decoder;
>  extern AVCodec ff_asv2_encoder;
> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
> new file mode 100644
> index 0000000000..59a1d7bf0a
> --- /dev/null
> +++ b/libavcodec/arbc.c
> @@ -0,0 +1,203 @@
> +/*
> + * Gryphon's Anim Compressor decoder
> + * Copyright (c) 2018 Paul B Mahol
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "libavutil/imgutils.h"
> +#include "libavutil/internal.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/mem.h"
> +
> +#include "avcodec.h"
> +#include "bytestream.h"
> +#include "internal.h"
> +
> +typedef struct ARBCContext {
> +    GetByteContext gb;
> +
> +    AVFrame *prev_frame;
> +} ARBCContext;
> +
> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame *frame)
> +{
> +    ARBCContext *s = avctx->priv_data;
> +    GetByteContext *gb = &s->gb;
> +    int nb_tiles = bytestream2_get_le16(gb);
> +    int h = avctx->height - 1;
> +
> +    for (int i = 0; i < nb_tiles; i++) {
> +        int y = bytestream2_get_byte(gb);
> +        int x = bytestream2_get_byte(gb);
> +        uint16_t mask = bytestream2_get_le16(gb);
> +        int start_y = y * 4, start_x = x * 4;
> +        int end_y = start_y + 4, end_x = start_x + 4;
> +
> +        for (int j = start_y; j < end_y; j++) {
> +            for (int k = start_x; k < end_x; k++) {
> +                if (mask & 0x8000) {
> +                    if (j >= avctx->height || k >= avctx->width)
> +                        continue;

this would treat 1 bits differently than 0 bits when they are outside
teh frame. That is 1 bits would not advance in the bitstream (aka not execute 
mask = mask << 1;
this also differs from fill_tileX()
is that intended ?

[...]
Paul B Mahol Jan. 23, 2019, 9:27 a.m. UTC | #11
On 1/22/19, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Tue, Jan 22, 2019 at 10:55:33AM +0100, Paul B Mahol wrote:
>> Thanks Kostya for great help in reversing binary.
>>
>> Signed-off-by: Paul B Mahol <onemda@gmail.com>
>> ---
>>  libavcodec/Makefile     |   1 +
>>  libavcodec/allcodecs.c  |   1 +
>>  libavcodec/arbc.c       | 203 ++++++++++++++++++++++++++++++++++++++++
>>  libavcodec/avcodec.h    |   1 +
>>  libavcodec/codec_desc.c |   7 ++
>>  libavformat/riff.c      |   1 +
>>  6 files changed, 214 insertions(+)
>>  create mode 100644 libavcodec/arbc.c
>>
>> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
>> index bf746c143d..8e240aecf0 100644
>> --- a/libavcodec/Makefile
>> +++ b/libavcodec/Makefile
>> @@ -198,6 +198,7 @@ OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
>>  OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
>>  OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
>>  OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
>> +OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
>>  OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
>>  OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
>>  OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
>> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
>> index fe0376e27e..5cbb09a5a4 100644
>> --- a/libavcodec/allcodecs.c
>> +++ b/libavcodec/allcodecs.c
>> @@ -41,6 +41,7 @@ extern AVCodec ff_anm_decoder;
>>  extern AVCodec ff_ansi_decoder;
>>  extern AVCodec ff_apng_encoder;
>>  extern AVCodec ff_apng_decoder;
>> +extern AVCodec ff_arbc_decoder;
>>  extern AVCodec ff_asv1_encoder;
>>  extern AVCodec ff_asv1_decoder;
>>  extern AVCodec ff_asv2_encoder;
>> diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
>> new file mode 100644
>> index 0000000000..59a1d7bf0a
>> --- /dev/null
>> +++ b/libavcodec/arbc.c
>> @@ -0,0 +1,203 @@
>> +/*
>> + * Gryphon's Anim Compressor decoder
>> + * Copyright (c) 2018 Paul B Mahol
>> + *
>> + * This file is part of FFmpeg.
>> + *
>> + * FFmpeg is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * FFmpeg is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with FFmpeg; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301 USA
>> + */
>> +
>> +#include <stdio.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +
>> +#include "libavutil/imgutils.h"
>> +#include "libavutil/internal.h"
>> +#include "libavutil/intreadwrite.h"
>> +#include "libavutil/mem.h"
>> +
>> +#include "avcodec.h"
>> +#include "bytestream.h"
>> +#include "internal.h"
>> +
>> +typedef struct ARBCContext {
>> +    GetByteContext gb;
>> +
>> +    AVFrame *prev_frame;
>> +} ARBCContext;
>> +
>> +static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame
>> *frame)
>> +{
>> +    ARBCContext *s = avctx->priv_data;
>> +    GetByteContext *gb = &s->gb;
>> +    int nb_tiles = bytestream2_get_le16(gb);
>> +    int h = avctx->height - 1;
>> +
>> +    for (int i = 0; i < nb_tiles; i++) {
>> +        int y = bytestream2_get_byte(gb);
>> +        int x = bytestream2_get_byte(gb);
>> +        uint16_t mask = bytestream2_get_le16(gb);
>> +        int start_y = y * 4, start_x = x * 4;
>> +        int end_y = start_y + 4, end_x = start_x + 4;
>> +
>> +        for (int j = start_y; j < end_y; j++) {
>> +            for (int k = start_x; k < end_x; k++) {
>> +                if (mask & 0x8000) {
>> +                    if (j >= avctx->height || k >= avctx->width)
>> +                        continue;
>
> this would treat 1 bits differently than 0 bits when they are outside
> teh frame. That is 1 bits would not advance in the bitstream (aka not
> execute
> mask = mask << 1;
> this also differs from fill_tileX()
> is that intended ?

No, well spotted, fixed locally.
diff mbox

Patch

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index bf746c143d..8e240aecf0 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -198,6 +198,7 @@  OBJS-$(CONFIG_APTX_HD_DECODER)         += aptx.o
 OBJS-$(CONFIG_APTX_HD_ENCODER)         += aptx.o
 OBJS-$(CONFIG_APNG_DECODER)            += png.o pngdec.o pngdsp.o
 OBJS-$(CONFIG_APNG_ENCODER)            += png.o pngenc.o
+OBJS-$(CONFIG_ARBC_DECODER)            += arbc.o
 OBJS-$(CONFIG_SSA_DECODER)             += assdec.o ass.o
 OBJS-$(CONFIG_SSA_ENCODER)             += assenc.o ass.o
 OBJS-$(CONFIG_ASS_DECODER)             += assdec.o ass.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index fe0376e27e..5cbb09a5a4 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -41,6 +41,7 @@  extern AVCodec ff_anm_decoder;
 extern AVCodec ff_ansi_decoder;
 extern AVCodec ff_apng_encoder;
 extern AVCodec ff_apng_decoder;
+extern AVCodec ff_arbc_decoder;
 extern AVCodec ff_asv1_encoder;
 extern AVCodec ff_asv1_decoder;
 extern AVCodec ff_asv2_encoder;
diff --git a/libavcodec/arbc.c b/libavcodec/arbc.c
new file mode 100644
index 0000000000..59a1d7bf0a
--- /dev/null
+++ b/libavcodec/arbc.c
@@ -0,0 +1,203 @@ 
+/*
+ * Gryphon's Anim Compressor decoder
+ * Copyright (c) 2018 Paul B Mahol
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "libavutil/imgutils.h"
+#include "libavutil/internal.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/mem.h"
+
+#include "avcodec.h"
+#include "bytestream.h"
+#include "internal.h"
+
+typedef struct ARBCContext {
+    GetByteContext gb;
+
+    AVFrame *prev_frame;
+} ARBCContext;
+
+static void fill_tile4(AVCodecContext *avctx, uint8_t *color, AVFrame *frame)
+{
+    ARBCContext *s = avctx->priv_data;
+    GetByteContext *gb = &s->gb;
+    int nb_tiles = bytestream2_get_le16(gb);
+    int h = avctx->height - 1;
+
+    for (int i = 0; i < nb_tiles; i++) {
+        int y = bytestream2_get_byte(gb);
+        int x = bytestream2_get_byte(gb);
+        uint16_t mask = bytestream2_get_le16(gb);
+        int start_y = y * 4, start_x = x * 4;
+        int end_y = start_y + 4, end_x = start_x + 4;
+
+        for (int j = start_y; j < end_y; j++) {
+            for (int k = start_x; k < end_x; k++) {
+                if (mask & 0x8000) {
+                    if (j >= avctx->height || k >= avctx->width)
+                        continue;
+                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 0] = color[0];
+                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 1] = color[1];
+                    frame->data[0][frame->linesize[0] * (h - j) + 3 * k + 2] = color[2];
+                }
+                mask = mask << 1;
+            }
+        }
+    }
+}
+
+static void fill_tileX(AVCodecContext *avctx, int tile_width, int tile_height,
+                       uint8_t *color, AVFrame *frame)
+{
+    ARBCContext *s = avctx->priv_data;
+    GetByteContext *gb = &s->gb;
+    const int step_h = tile_height / 4;
+    const int step_w = tile_width / 4;
+    int nb_tiles = bytestream2_get_le16(gb);
+    int h = avctx->height - 1;
+
+    for (int i = 0; i < nb_tiles; i++) {
+        int y = bytestream2_get_byte(gb);
+        int x = bytestream2_get_byte(gb);
+        uint16_t mask = bytestream2_get_le16(gb);
+        int start_y = y * tile_height, start_x = x * tile_width;
+        int end_y = start_y + tile_height, end_x = start_x + tile_width;
+
+        for (int j = start_y; j < end_y; j += step_h) {
+            for (int k = start_x; k < end_x; k += step_w) {
+                if (mask & 0x8000U) {
+                    for (int m = 0; m < step_h; m++) {
+                        for (int n = 0; n < step_w; n++) {
+                            if (j + m >= avctx->height || k + n >= avctx->width)
+                                continue;
+                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 0] = color[0];
+                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 1] = color[1];
+                            frame->data[0][frame->linesize[0] * (h - (j + m)) + 3 * (k + n) + 2] = color[2];
+                        }
+                    }
+                }
+                mask = mask << 1;
+            }
+        }
+    }
+}
+
+static int decode_frame(AVCodecContext *avctx, void *data,
+                        int *got_frame, AVPacket *avpkt)
+{
+    ARBCContext *s = avctx->priv_data;
+    AVFrame *const frame = data;
+    int ret, nb_segments, keyframe = 1;
+
+    if (avpkt->size < 1)
+        return AVERROR_INVALIDDATA;
+
+    if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+        return ret;
+
+    if (s->prev_frame->data[0]) {
+        ret = av_frame_copy(frame, s->prev_frame);
+        if (ret < 0)
+            return ret;
+    }
+
+    bytestream2_init(&s->gb, avpkt->data, avpkt->size);
+    bytestream2_skip(&s->gb, 8);
+    nb_segments = bytestream2_get_le16(&s->gb);
+    if (nb_segments == 0)
+        keyframe = 0;
+
+    for (int i = 0; i < nb_segments; i++) {
+        int resolution_flag;
+        uint8_t fill[3];
+
+        if (bytestream2_get_bytes_left(&s->gb) <= 0)
+            return AVERROR_INVALIDDATA;
+
+        fill[0] = bytestream2_get_byte(&s->gb);
+        bytestream2_skip(&s->gb, 1);
+        fill[1] = bytestream2_get_byte(&s->gb);
+        bytestream2_skip(&s->gb, 1);
+        fill[2] = bytestream2_get_byte(&s->gb);
+        bytestream2_skip(&s->gb, 1);
+        resolution_flag = bytestream2_get_byte(&s->gb);
+
+        if (resolution_flag & 0x10)
+            fill_tileX(avctx, 1024, 1024, fill, frame);
+        if (resolution_flag & 0x08)
+            fill_tileX(avctx, 256, 256, fill, frame);
+        if (resolution_flag & 0x04)
+            fill_tileX(avctx, 64, 64, fill, frame);
+        if (resolution_flag & 0x02)
+            fill_tileX(avctx, 16, 16, fill, frame);
+        if (resolution_flag & 0x01)
+            fill_tile4(avctx, fill, frame);
+    }
+
+    av_frame_unref(s->prev_frame);
+    if ((ret = av_frame_ref(s->prev_frame, frame)) < 0)
+        return ret;
+
+    frame->pict_type = keyframe ? AV_PICTURE_TYPE_I : AV_PICTURE_TYPE_P;
+    frame->key_frame = keyframe;
+    *got_frame = 1;
+
+    return avpkt->size;
+}
+
+static av_cold int decode_init(AVCodecContext *avctx)
+{
+    ARBCContext *s = avctx->priv_data;
+
+    avctx->pix_fmt = AV_PIX_FMT_RGB24;
+
+    s->prev_frame = av_frame_alloc();
+    if (!s->prev_frame)
+        return AVERROR(ENOMEM);
+
+    return 0;
+}
+
+static av_cold int decode_close(AVCodecContext *avctx)
+{
+    ARBCContext *s = avctx->priv_data;
+
+    av_frame_free(&s->prev_frame);
+
+    return 0;
+}
+
+AVCodec ff_arbc_decoder = {
+    .name           = "arbc",
+    .long_name      = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
+    .type           = AVMEDIA_TYPE_VIDEO,
+    .id             = AV_CODEC_ID_ARBC,
+    .priv_data_size = sizeof(ARBCContext),
+    .init           = decode_init,
+    .decode         = decode_frame,
+    .close          = decode_close,
+    .capabilities   = AV_CODEC_CAP_DR1,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE |
+                      FF_CODEC_CAP_INIT_CLEANUP,
+};
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 90f9f08289..72dc277dfd 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -453,6 +453,7 @@  enum AVCodecID {
     AV_CODEC_ID_WCMV,
     AV_CODEC_ID_RASC,
     AV_CODEC_ID_HYMT,
+    AV_CODEC_ID_ARBC,
 
     /* various PCM "codecs" */
     AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing at the start of audio codecs
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 10a639101c..7b254c1d45 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -1698,6 +1698,13 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .long_name = NULL_IF_CONFIG_SMALL("HuffYUV MT"),
         .props     = AV_CODEC_PROP_INTRA_ONLY | AV_CODEC_PROP_LOSSLESS,
     },
+    {
+        .id        = AV_CODEC_ID_ARBC,
+        .type      = AVMEDIA_TYPE_VIDEO,
+        .name      = "arbc",
+        .long_name = NULL_IF_CONFIG_SMALL("Gryphon's Anim Compressor"),
+        .props     = AV_CODEC_PROP_LOSSY,
+    },
 
     /* various PCM "codecs" */
     {
diff --git a/libavformat/riff.c b/libavformat/riff.c
index d5a509c8b9..8f0fd99e22 100644
--- a/libavformat/riff.c
+++ b/libavformat/riff.c
@@ -476,6 +476,7 @@  const AVCodecTag ff_codec_bmp_tags[] = {
     { AV_CODEC_ID_WCMV,         MKTAG('W', 'C', 'M', 'V') },
     { AV_CODEC_ID_RASC,         MKTAG('R', 'A', 'S', 'C') },
     { AV_CODEC_ID_HYMT,         MKTAG('H', 'Y', 'M', 'T') },
+    { AV_CODEC_ID_ARBC,         MKTAG('A', 'R', 'B', 'C') },
     { AV_CODEC_ID_NONE,         0 }
 };