[FFmpeg-devel] [PATCH] avcodec: add QOI decoder and demuxer and parser and encoder and muxer

[Tomas Härdin]

tor 2022-06-02 klockan 12:59 +0200 skrev Paul B Mahol:
> +++ b/libavcodec/qoidec.c
> 
> +#define QOI_OP_INDEX  0x00 /* 00xxxxxx */
> +#define QOI_OP_DIFF   0x40 /* 01xxxxxx */
> +#define QOI_OP_LUMA   0x80 /* 10xxxxxx */
> +#define QOI_OP_RUN    0xc0 /* 11xxxxxx */
> +#define QOI_OP_RGB    0xfe /* 11111110 */
> +#define QOI_OP_RGBA   0xff /* 11111111 */
> +
> +#define QOI_MASK_2    0xc0 /* 11000000 */
> +
> +#define QOI_COLOR_HASH(px) (px[0]*3 + px[1]*5 + px[2]*7 + px[3]*11)

Put these in a common header

> +static int qoi_decode_frame(AVCodecContext *avctx, AVFrame *p,
> +                            int *got_frame, AVPacket *avpkt)
> +{
> +    const uint8_t *buf = avpkt->data;
> +    int ret, buf_size = avpkt->size;
> +    int width, height, run = 0;
> +    uint8_t index[64][4] = { 0 };

I think some compilers require {{0}}

> +    uint8_t px[4] = { 0, 0, 0, 255 };
> +    GetByteContext gb;
> +    uint8_t *dst;
> +    uint64_t len;
> +
> +    if (buf_size < 20)
> +        return AVERROR_INVALIDDATA;
> +
> +    bytestream2_init(&gb, buf, buf_size);
> +    bytestream2_skip(&gb, 4);
> +    width  = bytestream2_get_be32(&gb);
> +    height = bytestream2_get_be32(&gb);
> +    bytestream2_skip(&gb, 2);

This should reject linear RGB, not silently treat it as sRGB.

> +
> +    if ((ret = ff_set_dimensions(avctx, width, height)) < 0)
> +        return ret;
> +
> +    if ((ret = av_image_check_size(avctx->width, avctx->height, 0,
> NULL)) < 0)
> +        return ret;

This call looks unnecessary as ff_set_dimensions() calls
av_image_check_size2()

> +    avctx->pix_fmt = AV_PIX_FMT_RGBA;

Still not a fan of this. Now users can encode RGB yet they unexpectedly
get RGBA back.

> +++ b/libavcodec/qoienc.c
>
> +static int qoi_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> +                            const AVFrame *pict, int *got_packet)
> +{
> [...]
> +                index_pos = QOI_COLOR_HASH(px) & 63;
> +
> +                if (!memcmp(index[index_pos], px, 4)) {
> +                    bytestream_put_byte(&buf, QOI_OP_INDEX |
> index_pos);

This needs protection against outputting 0x0000000000000001

/Tomas