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

[Tomas Härdin]

tor 2022-06-02 klockan 14:06 +0200 skrev Paul B Mahol:
> On Thu, Jun 2, 2022 at 1:58 PM Tomas Härdin <tjoppen at acc.umu.se>
> wrote:
> > 
> > > +    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?

>From the spec:

>    uint8_t  colorspace; // 0 = sRGB with linear alpha
>                         // 1 = all channels linear

RGB in lavc is always sRGB as far as I'm aware.
> 

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

I had the notion that the encoder could produce runs of index_pos == 0
followed by a single index_pos == 1, thus accidentally signalling end
of image. But the RLE stuff ensures that can't happen. The closest we
can get is an QOI_OP_RGBA with {0,0,0,0} followed by QOI_OP_INDEX with
index_pos == 0 which is five 0x00 in a row. A further match for
index_pos == 0 would be handled as a run. So there's no risk of
accidentally marking end of image. This is why the spec says an encoder
must not output consecutive QOI_OP_INDEX with the same index.

/Tomas