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

[Paul B Mahol <onemda@gmail.com>]

On Thu, Jun 2, 2022 at 1:58 PM Tomas Härdin <tjoppen@acc.umu.se> wrote:

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

Reject?


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

OK

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

What?


>
> /Tomas
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".