[FFmpeg-devel] [PATCH 1/2] avcodec: add photocd decoder

[Peter Ross]

On Wed, Dec 19, 2018 at 09:32:03PM +0100, Paul B Mahol wrote:
> Signed-off-by: Paul B Mahol <onemda at gmail.com>
> ---
>  libavcodec/Makefile     |   1 +
>  libavcodec/allcodecs.c  |   1 +
>  libavcodec/avcodec.h    |   1 +
>  libavcodec/codec_desc.c |   7 +
>  libavcodec/photocd.c    | 445 ++++++++++++++++++++++++++++++++++++++++

here is a small review.

> +    int      thumbnails;  //* number of thumbnails; 0 for normal image */

'//*' strange comment style

> +static const uint32_t img_start [] = { 0 /*dummy */ , 8192, 47104, 196608 };
> +static const uint16_t def_width [] = { 0 /*dummy */ , 192, 384, 768, 1536, 3072, 6144 };
> +static const uint16_t def_height[] = { 0 /*dummy */ , 128, 256, 512, 1024, 2048, 4096 };

if this was a struct it would be more reable. the array index is the same quality.

struct {
   uint32_t start;
   uint16_t width, height;
} img_info[];

also not obvious to me why the dummy values are needed.
array index is alway take from s->resolution, and that is av_clip'd to [1,5].

> +static int photocd_decode_frame(AVCodecContext *avctx, void *data,
> +                                int *got_frame, AVPacket *avpkt)
> +{
> +    PhotoCDContext *s = avctx->priv_data;
> +    const uint8_t *buf = avpkt->data;
> +    GetByteContext *gb = &s->gb;
> +    AVFrame *p = data;
> +    uint8_t *ptr, *ptr1, *ptr2;
> +    int ret;
> +
> +    if (avpkt->size < 7)
> +        return AVERROR_INVALIDDATA;
> +
> +    bytestream2_init(gb, avpkt->data, avpkt->size);

could delay bytestream2_init further down, as it's not used yet.

> +    if (!strncmp("PCD_OPA", buf, 7)) {
> +        s->thumbnails = AV_RL16(buf + 10);

need to check avpkt->size before buf[10];

> +        av_log(avctx, AV_LOG_WARNING, "this is a thumbnails file, "
> +               "reading first thumbnail only\n");
> +    } else if (avpkt->size < 786432) {
> +        return AVERROR_INVALIDDATA;
> +    }
> +
> +    s->orientation = s->thumbnails ? buf[12] & 3 : buf[0x48] & 3;

need to check avpkt->size here too.

also where can i find a sample to fuzz test?

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20181220/9094d00b/attachment.sig>