[FFmpeg-devel] [PATCH] DeluxePaint Animation playback system
Michael Niedermayer
michaelni
Mon Oct 19 22:36:11 CEST 2009
On Sun, Oct 18, 2009 at 04:19:04PM +1100, Peter Ross wrote:
> On Thu, Oct 15, 2009 at 07:01:28PM +0200, Diego Biurrun wrote:
> > On Thu, Oct 15, 2009 at 07:54:05PM +1100, Peter Ross wrote:
> > >
> > > +++ b/libavcodec/anm.c
> > > @@ -0,0 +1,177 @@
> > > +static av_cold int decode_init(AVCodecContext *avctx) {
> >
> > K&R function declarations please; you use them everywhere else.
>
> Ok.
[...]
> +static av_cold int decode_init(AVCodecContext *avctx)
> +{
> + AnmContext *s = avctx->priv_data;
> + const uint8_t *buf;
> + int i;
> +
> + avctx->pix_fmt = PIX_FMT_PAL8;
> +
> + if (avctx->extradata_size != 16*8 + 4*256)
> + return -1;
> +
> + s->frame.reference = 1;
> + if (avctx->get_buffer(avctx, &s->frame) < 0) {
> + av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> + return -1;
> + }
> +
> + buf = avctx->extradata + 16*8;
> + for (i = 0; i < 256; i++)
> + ((int*)s->frame.data[1])[i] = bytestream_get_le32(&buf);
i think this should be uint32_t not int
> +
> + return 0;
> +}
> +
> +/**
> + * Perform decode operation
> + * @param dst Destination image buffer
> + * @param buf Source buffer (optional, see below)
> + * @param pixel Fill color (optional, see below)
> + * @param count Pixel count
> + * @param x Pointer to x-axis counter
> + * @param width Image width
> + * @param linesize Destination imaage buffer linesize
^^
typo
[...]
> +static int decode_frame(AVCodecContext *avctx,
> + void *data, int *data_size,
> + AVPacket *avpkt)
> +{
> + AnmContext *s = avctx->priv_data;
> + const uint8_t *buf = avpkt->data;
> + const int buf_size = avpkt->size;
> + const uint8_t *buf_end = buf + buf_size;
> + uint8_t *dst = s->frame.data[0];
> + uint8_t *dst_end = s->frame.data[0] + s->frame.linesize[0]*avctx->height;
> + int count;
> +
> + if (buf[0] != 0x42) {
> + av_log_ask_for_sample(avctx, "unknown record type\n");
> + return buf_size;
> + }
> + if (buf[1]) {
> + av_log_ask_for_sample(avctx, "padding bytes not supported\n");
> + return buf_size;
> + }
> + buf += 4;
> +
> + s->x = 0;
> + do {
> + /* if statements are ordered by probability */
> +#define OP(buf, pixel, count) \
> + op(&dst, dst_end, (buf), buf_end, (pixel), (count), &s->x, avctx->width, s->frame.linesize[0])
> +
this is missing a reget_buffer()
> + int type = bytestream_get_byte(&buf);
> + count = type & 0x7F;
> + type >>= 7;
> + if (count) {
> + OP(type ? NULL : &buf, -1, count);
> + } else if (!type) {
> + int pixel;
> + count = bytestream_get_byte(&buf); /* count==0 gives nop */
> + pixel = bytestream_get_byte(&buf);
> + OP(NULL, pixel, count);
> + } else {
> + int pixel;
> + type = bytestream_get_le16(&buf);
> + count = type & 0x3FFF;
> + type >>= 14;
> + if (!count) {
> + if (type == 0)
> + break; // stop
> + if (type == 2) {
> + av_log_ask_for_sample(avctx, "unknown opcode");
> + return AVERROR_INVALIDDATA;
> + }
> + continue;
> + }
> + pixel = type == 3 ? bytestream_get_byte(&buf) : -1;
> + if (type == 1) count += 0x4000;
> + OP(type == 2 ? &buf : NULL, pixel, count);
> + }
> + } while (dst < dst_end && buf + 1 < buf_end);
this code seems to assume linesize > 0 but it could also be < 0
[...]
> +typedef struct {
> + int base_record;
> + int nb_records;
> + int size;
> + uint16_t *record_sizes;
why is this kept for each page?
is seeking possible in this format? if not it seems to make no sense to keep
the previous
> +} Page;
[...]
> +
> +static int read_header(AVFormatContext *s,
> + AVFormatParameters *ap)
> +{
> + AnmDemuxContext *anm = s->priv_data;
> + ByteIOContext *pb = s->pb;
> + AVStream *st;
> + int i, ret;
> +
> + url_fskip(pb, 4); /* magic number */
> + if (get_le16(pb) != MAX_PAGES) {
> + av_log_ask_for_sample(s, "max_pages != " AV_STRINGIFY(MAX_PAGES) "\n");
> + return AVERROR_INVALIDDATA;
> + }
> +
> + anm->nb_pages = get_le16(pb);
> + anm->nb_records = get_le32(pb);
the field in anm is a signed int of possibly just 32bit so it could end
negative
> + url_fskip(pb, 2); /* max records per page */
> + anm->page_table_offset = get_le16(pb);
> + if (get_le32(pb) != ANIM_TAG)
> + return AVERROR_INVALIDDATA;
> +
> + /* video stream */
> + st = av_new_stream(s, 0);
> + if (!st)
> + return AVERROR(ENOMEM);
> + st->codec->codec_type = CODEC_TYPE_VIDEO;
> + st->codec->codec_id = CODEC_ID_ANM;
> + st->codec->codec_tag = 0; /* no fourcc */
> + st->codec->width = get_le16(pb);
> + st->codec->height = get_le16(pb);
> + if (get_byte(pb) != 0) {
> + av_log_ask_for_sample(s, "unknown variant");
> + av_close_input_stream(s);
> + return AVERROR_INVALIDDATA;
> + }
> + url_fskip(pb, 1); /* frame rate multiplier info */
> +
> + /* ignore last delta record (used for looping) */
> + if (get_byte(pb)) /* has_last_delta */
> + anm->nb_records = FFMAX(anm->nb_records - 1, 0);
> +
> + url_fskip(pb, 1); /* last_delta_valid */
> +
> + if (get_byte(pb) != 0) {
> + av_log_ask_for_sample(s, "unknown pixel type");
> + av_close_input_stream(s);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + if (get_byte(pb) != 1) {
> + av_log_ask_for_sample(s, "unknown compression type");
> + av_close_input_stream(s);
> + return AVERROR_INVALIDDATA;
could be simplified with a goto
> + }
> +
> + url_fskip(pb, 1); /* other recs per frame */
> +
> + if (get_byte(pb) != 1) {
> + av_log_ask_for_sample(s, "unknown bitmap type");
> + av_close_input_stream(s);
> + return AVERROR_INVALIDDATA;
> + }
> +
> + url_fskip(pb, 32); /* record_types */
> + url_fskip(pb, 4); /* nb_frames */
AVStream has a nb_frames field that maybe could be set to this assuming its
what i think it is
[...]
> +static int read_packet(AVFormatContext *s,
> + AVPacket *pkt)
> +{
> + AnmDemuxContext *anm = s->priv_data;
> + ByteIOContext *pb = s->pb;
> + Page *p;
> + int j;
> +
> + if (url_feof(s->pb))
> + return AVERROR(EIO);
> +
> + if (anm->page < 0)
> + return 0;
> +
> +repeat:
maybe this could be a do/while loop which if it has no other disadvantages
and doesnt need more lines would IMHO be clearer than a goto
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091019/1fe43eee/attachment.pgp>
More information about the ffmpeg-devel
mailing list