On 1/22/2019 11:54 AM, Paul B Mahol wrote: > On 1/22/19, James Almer <jamr...@gmail.com> wrote: >> On 1/22/2019 10:30 AM, Paul B Mahol wrote: >>> On 1/22/19, James Almer <jamr...@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 <one...@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel