On Sun, Oct 13, 2013 at 07:24:25PM +0200, David Kment wrote:
> Hi,
> 
> I developed an HNM4/HNM4A demuxer & video decoder for libav a while
> back, and now cleaned it up.
> Samples can be found here:
> http://samples.mplayerhq.hu/game-formats/hnm/losteden-hnm4/
> I have more samples, i can upload those if needed.
> 
> First time contributor, so if the format of the patch is wrong, or
> something in the code, just tell me.

Since it was sent inline, your mailer has mangled it. Please attach it as a
file to the message next time.

> Also I think it should work on big endian platforms, but couldnt
> test because i dont have access to a BE device.
> 

that's a common story
But if the code is sane then it should work there indeed (and that will be
tested eventually).

> 
>  libavcodec/Makefile      |   1 +
>  libavcodec/allcodecs.c   |   1 +
>  libavcodec/avcodec.h     |   1 +
>  libavcodec/hnm4video.c   | 483
> +++++++++++++++++++++++++++++++++++++++++++++++
>  libavformat/Makefile     |   1 +
>  libavformat/allformats.c |   1 +
>  libavformat/hnm.c        | 203 ++++++++++++++++++++
>  7 files changed, 691 insertions(+)
> 
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index 8e0d60d..f64a56f 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -197,6 +197,7 @@ OBJS-$(CONFIG_H264_DECODER)            += h264.o
> \
>                                            h264_loopfilter.o
> h264_direct.o      \
>                                            cabac.o h264_sei.o
> h264_ps.o         \
>                                            h264_refs.o h264_cavlc.o
> h264_cabac.o
> +OBJS-$(CONFIG_HNM4_VIDEO_DECODER)      += hnm4video.o
>  OBJS-$(CONFIG_HUFFYUV_DECODER)         += huffyuv.o huffyuvdec.o
>  OBJS-$(CONFIG_HUFFYUV_ENCODER)         += huffyuv.o huffyuvenc.o
>  OBJS-$(CONFIG_IAC_DECODER)             += imc.o
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index 55d7957..b62efb9 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -153,6 +153,7 @@ void avcodec_register_all(void)
>      REGISTER_DECODER(H263I,             h263i);
>      REGISTER_ENCODER(H263P,             h263p);
>      REGISTER_DECODER(H264,              h264);
> +    REGISTER_DECODER(HNM4_VIDEO,        hnm4_video);
>      REGISTER_ENCDEC (HUFFYUV,           huffyuv);
>      REGISTER_DECODER(IDCIN,             idcin);
>      REGISTER_DECODER(IFF_BYTERUN1,      iff_byterun1);
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index d535308..ae0bfe2 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -274,6 +274,7 @@ enum AVCodecID {
>      AV_CODEC_ID_ESCAPE130,
>      AV_CODEC_ID_G2M,
>      AV_CODEC_ID_WEBP,
> +    AV_CODEC_ID_HNM4_VIDEO,
> 
>      /* various PCM "codecs" */
>      AV_CODEC_ID_FIRST_AUDIO = 0x10000,     ///< A dummy id pointing
> at the start of audio codecs
> diff --git a/libavcodec/hnm4video.c b/libavcodec/hnm4video.c
> new file mode 100644
> index 0000000..d8d0c39
> --- /dev/null
> +++ b/libavcodec/hnm4video.c
> @@ -0,0 +1,483 @@
> +/*
> + * Cryo Interactive Entertainment HNM4 video decoder
> + * Copyright (c) 2012 David Kment
> + *
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +#include "avcodec.h"
> +#include "internal.h"
> +#include "libavutil/internal.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/mem.h"
> +#include <string.h>
> +
> +#define HNM4_CHUNK_ID_PL 19536
> +#define HNM4_CHUNK_ID_IZ 23113
> +#define HNM4_CHUNK_ID_IU 21833
> +#define HNM4_CHUNK_ID_SD 17491
> +
> +typedef struct Hnm4VideoContext {
> +
> +    uint8_t version;
> +    uint16_t width;
> +    uint16_t height;
> +
> +    uint8_t *current;
> +    uint8_t *previous;
> +    uint8_t *buffer1;
> +    uint8_t *buffer2;
> +    uint8_t *processed;
> +    uint8_t *palette;
> +} Hnm4VideoContext;
> +
> +static int getbit(uint8_t *src, uint32_t *brBitsRemaining, uint32_t
> *brOffset, uint32_t *brQueue) {
> +
> +    if (*brBitsRemaining == 0) {
> +
> +        *brQueue = AV_RL32(src + *brOffset);
> +        *brOffset += 4;
> +        *brBitsRemaining = 32;
> +    }
> +
> +    (*brBitsRemaining)--;
> +
> +    if (*brQueue & 0x80000000) {
> +        *brQueue <<= 1;
> +        return 1;
> +    }
> +    else {
> +        *brQueue <<= 1;
> +        return 0;
> +    }
> +}

first, it would be much better to use bytestream2 like:

#include "bytestream.h"

static int getbit(GetByteContext *gb, uint32_t *bitbuf, int *bits)
{
    int ret;

    if (!*bits) {
        *bitbuf = bytestream2_get_le32(gb);
        *bits   = 32;
    }

    ret = *bitbuf >> 31;
    bitbuf <<= 1;
    *bits--;

    return ret;
}

Otherwise you need to track source pointer and position all around and might
miss it somewhere.

> +
> +static void unpack_intraframe(Hnm4VideoContext *hnm, uint8_t *src) {
> +
> +    uint32_t brBitsRemaining;
> +    uint32_t brOffset;
> +    uint32_t brQueue;
> +
> +    uint32_t outputOffset, count, offset;
> +    uint16_t word;
> +
> +    outputOffset = 0;
> +    brBitsRemaining = 0;
> +    brQueue = 0;
> +    brOffset = 0;
> +
> +    while(1) {
> +
> +        if (getbit(src, &brBitsRemaining, &brOffset, &brQueue)) {
> +
> +            hnm->current[outputOffset++] = src[brOffset++];
> +
> +        } else {
> +
> +            if (getbit(src, &brBitsRemaining, &brOffset, &brQueue)) {
> +
> +                word = src[brOffset++];
> +                word += src[brOffset++] * 256;

would be nice to replace with bytestream2_get_le16()

> +                count = word & 0x07;
> +                offset = 0xffffe000 | (word >> 3);
> +
> +                if (!count) {count = src[brOffset++];}

that one looks evil - it's better to declare
int offset;
...
offset = sign_extend(word >> 3, 13);

so it will produce signed value from given number of bits without such hacks.


> +                if (!count) {return;}
> +
> +            } else {
> +                count = getbit(src, &brBitsRemaining, &brOffset,
> &brQueue) << 1;
> +                count |= getbit(src, &brBitsRemaining, &brOffset,
> &brQueue);
> +
> +                offset = 0xffffff00 | src[brOffset++];
> +            }
> +
> +            count += 2;
> +
> +            offset += outputOffset;
> +
> +            while (count--) {
> +                hnm->current[outputOffset++] = hnm->current[offset++];
> +            }

And of course a check is needed that you don't overread bytes or read them
from the wrong position. It won't happen with normal files but will happen
with damaged ones for sure (and we care).

> +        }
> +    }
> +}
> +
> +static void postprocess_current_frame(Hnm4VideoContext *hnm) {
> +
> +    uint32_t x, y;
> +    uint32_t srcX, srcY;
> +
> +    for(y = 0; y < hnm->height; y++) {
> +        srcY = y - (y % 2);
> +        srcX = srcY * hnm->width + (y % 2);
> +        for(x = 0; x < hnm->width; x++) {
> +            hnm->processed[(y * hnm->width) + x] = hnm->current[srcX];
> +            srcX += 2;
> +        }
> +    }
> +}
> +
> +static void copy_processed_frame(Hnm4VideoContext *hnm, AVFrame *frame) {
> +
> +    uint32_t x, y, srcX;
> +
> +    for(y = 0; y < hnm->height; y++) {
> +        srcX = y * hnm->width;
> +        for(x = 0; x < hnm->width; x++) {
> +            frame->data[0][(y * frame->linesize[0]) + x] =
> hnm->processed[srcX++];
> +        }

uint8_t *src = hnm->processed;
uint8_t *dst = frame->data[0];
int y;

for (y = 0; y < hnm->height; y++) {
    memcpy(dst, src, hnm->width);
    src += hnm->width;
    dst += frame->linesize[0];
}

> +    }
> +}
> +
> +static void decode_interframe_v4(Hnm4VideoContext *hnm, uint8_t *src) {
> +
> +    uint32_t readOffset, writeOffset;
> +    uint8_t tag;
> +    uint32_t count, left;
> +    uint8_t previous, backline, backward, swap;
> +    int32_t offset;
> +
> +    readOffset = 0;
> +    writeOffset = 0;
> +
> +    while(1) {
> +
> +        count = src[readOffset] & 0x1F;
> +
> +        if(count == 0) {
> +
> +            tag = src[readOffset++] & 0xE0;
> +            tag = tag >> 5;
> +
> +            if(tag == 0) {
> +                hnm->current[writeOffset++] = src[readOffset++];
> +                hnm->current[writeOffset++] = src[readOffset++];
> +            } else if(tag == 1) {
> +                writeOffset += src[readOffset++] * 2;
> +            } else if(tag == 2) {
> +                count = src[readOffset++];
> +                count += src[readOffset++] * 256;
> +                count *= 2;
> +                writeOffset += count;
> +            } else if(tag == 3) {
> +                count = src[readOffset++] * 2;
> +                while(count > 0) {
> +                    hnm->current[writeOffset++] = src[readOffset];
> +                    count--;
> +                }
> +                readOffset++;
> +            } else {
> +                break;
> +            }
> +
> +        } else {
> +
> +            previous = src[readOffset] & 0x20;
> +            backline = src[readOffset] & 0x40;
> +            backward = src[readOffset] & 0x80;
> +            swap = src[readOffset+1] & 0x01;
> +
> +            offset = src[readOffset+1];
> +            offset += src[readOffset+2] * 256;
> +            offset = (offset >> 1) & 0x7FFF;
> +            offset = writeOffset + (offset * 2) - 0x8000;
> +
> +            left = count;
> +
> +            if(previous) {
> +                while(left > 0) {
> +                    if(backline) {
> +                        hnm->current[writeOffset++] =
> hnm->previous[offset-(2 * hnm->width)+1];
> +                        hnm->current[writeOffset++] =
> hnm->previous[offset++];
> +                        offset++;
> +                    } else {
> +                        hnm->current[writeOffset++] =
> hnm->previous[offset++];
> +                        hnm->current[writeOffset++] =
> hnm->previous[offset++];
> +                    }
> +                    if(backward) {offset -= 4;}
> +                    left--;
> +                }
> +            } else {
> +                while(left > 0) {
> +                    if(backline) {
> +                        hnm->current[writeOffset++] =
> hnm->current[offset-(2 * hnm->width)+1];
> +                        hnm->current[writeOffset++] =
> hnm->current[offset++];
> +                        offset++;
> +                    } else {
> +                        hnm->current[writeOffset++] =
> hnm->current[offset++];
> +                        hnm->current[writeOffset++] =
> hnm->current[offset++];
> +                    }
> +                    if(backward) {offset -= 4;}
> +                    left--;
> +                }
> +            }

this code also needs checks if you're reading from valid memory region and
write to one too

> +
> +            if(swap) {
> +                left = count;
> +                writeOffset -= count * 2;
> +                while(left > 0) {
> +                    swap = hnm->current[writeOffset];
> +                    hnm->current[writeOffset] =
> hnm->current[writeOffset + 1];
> +                    hnm->current[writeOffset + 1] = swap;
> +                    left--;
> +                    writeOffset += 2;
> +                }
> +            }
> +
> +            readOffset += 3;
> +        }
> +    }
> +}
> +
> +static void decode_interframe_v4a(Hnm4VideoContext *hnm, uint8_t *src) {
> +
> +    uint32_t readOffset, writeOffset;
> +    uint8_t tag, count;
> +    uint8_t previous, delta;
> +    uint32_t offset;
> +
> +    readOffset = 0;
> +    writeOffset = 0;
> +
> +    while(1) {
> +
> +        count = src[readOffset] & 0x3F;
> +
> +        if(count == 0) {
> +
> +            tag = src[readOffset++] & 0xC0;
> +            tag = tag >> 6;
> +
> +            if(tag == 0) {
> +                writeOffset += src[readOffset++];
> +            } else if(tag == 1) {
> +                hnm->current[writeOffset] = src[readOffset++];
> +                hnm->current[writeOffset + hnm->width] = src[readOffset++];
> +                writeOffset++;
> +            } else if(tag == 2) {
> +                writeOffset += hnm->width;
> +            } else if(tag == 3) {
> +                break;
> +            }
> +
> +        } else {
> +
> +            delta = src[readOffset] & 0x80;
> +            previous = src[readOffset] & 0x40;
> +
> +            offset = writeOffset;
> +            offset += src[readOffset + 1];
> +            offset += src[readOffset + 2] * 256;
> +
> +            if(delta) {offset -= 0x10000;}
> +
> +            if(previous) {
> +                while(count > 0) {
> +                    hnm->current[writeOffset] = hnm->previous[offset];
> +                    hnm->current[writeOffset + hnm->width] =
> hnm->previous[offset + hnm->width];
> +                    writeOffset++;
> +                    offset++;
> +                    count--;
> +                }
> +            } else {
> +                while(count > 0) {
> +                    hnm->current[writeOffset] = hnm->current[offset];
> +                    hnm->current[writeOffset + hnm->width] =
> hnm->current[offset + hnm->width];
> +                    writeOffset++;
> +                    offset++;
> +                    count--;
> +                }
> +            }
> +
> +            readOffset += 3;
> +        }
> +    }
> +}
> +
> +static void hnm_update_palette(Hnm4VideoContext *hnm, uint8_t *src) {
> +
> +    uint16_t start, count;
> +    uint8_t r, g, b;
> +    uint32_t readOffset, writeOffset;
> +    int eightBitColors;
> +
> +    readOffset = 8; // skip first 8 bytes
> +    writeOffset = 0;
> +
> +    eightBitColors = (src[7] & 0x80 && hnm->version == 0x4a);
> +
> +    while(1) {
> +
> +        start = src[readOffset++];
> +        count = src[readOffset++];
> +
> +        if(start == 255 && count == 255) {break;}
> +
> +        if(count == 0) {count = 256;}
> +
> +        writeOffset = start * 4;
> +
> +        while(count > 0) {
> +            r = src[readOffset++];
> +            g = src[readOffset++];
> +            b = src[readOffset++];
> +
> +            if(!eightBitColors) {
> +                r <<= 2;
> +                g <<= 2;
> +                b <<= 2;
> +            }
> +
> +            hnm->palette[writeOffset++] = b;
> +            hnm->palette[writeOffset++] = g;
> +            hnm->palette[writeOffset++] = r;
> +            hnm->palette[writeOffset++] = 0;
> +            count--;

I'd recommend (BE-proof too) way of storing palette like we do:
as uint32_t pal[256];
and read it with bytestream2 like

hnm->palette[i] = bytestream2_get_be24(gb);
if (!eightBitColors)
  hnm->palette[i] <<= 2;

> +        }
> +    }
> +}
> +
> +static void hnm_flip_buffers(Hnm4VideoContext *hnm) {
> +
> +    uint8_t *temp;
> +
> +    temp = hnm->current;
> +    hnm->current = hnm->previous;
> +    hnm->previous = temp;
> +}
> +
> +static int hnm_decode_frame(AVCodecContext *avctx, void *data,
> +                            int *got_frame, AVPacket *avpkt) {
> +
> +    AVFrame *frame = data;
> +    Hnm4VideoContext *hnm = avctx->priv_data;
> +    int ret;
> +    uint16_t chunk_id;
> +
> +    if ((ret = ff_get_buffer(avctx, frame, 0)) < 0) {
> +        av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
> +        return ret;
> +    }
> +
> +    chunk_id = AV_RL16(avpkt->data + 4);
> +
> +    if(chunk_id == HNM4_CHUNK_ID_PL) {
> +
> +        hnm_update_palette(hnm, avpkt->data);
> +    }
> +    else if(chunk_id == HNM4_CHUNK_ID_IZ) {
> +
> +        unpack_intraframe(hnm, avpkt->data + 12);
> +
> +        memcpy(hnm->previous, hnm->current, hnm->width * hnm->height);
> +
> +        if(hnm->version == 0x4a) {
> +            memcpy(hnm->processed, hnm->current, hnm->width * hnm->height);
> +        } else {
> +            postprocess_current_frame(hnm);
> +        }
> +
> +        copy_processed_frame(hnm, frame);
> +
> +        frame->pict_type = AV_PICTURE_TYPE_I;
> +        frame->key_frame = 1;
> +
> +        frame->palette_has_changed = 1;

you need to set that only when you update palette really

> +        memcpy(frame->data[1], hnm->palette, 256 * 4);
> +
> +        *got_frame = 1;
> +    }
> +    else if(chunk_id == HNM4_CHUNK_ID_IU) {
> +
> +        if(hnm->version == 0x4a) {
> +            decode_interframe_v4a(hnm, avpkt->data + 8);
> +            memcpy(hnm->processed, hnm->current, hnm->width * hnm->height);
> +        } else {
> +            decode_interframe_v4(hnm, avpkt->data + 8);
> +            postprocess_current_frame(hnm);
> +        }
> +
> +        copy_processed_frame(hnm, frame);
> +
> +        frame->pict_type = AV_PICTURE_TYPE_P;
> +        frame->key_frame = 0;
> +
> +        frame->palette_has_changed = 1;
> +        memcpy(frame->data[1], hnm->palette, 256 * 4);
> +
> +        *got_frame = 1;
> +
> +        hnm_flip_buffers(hnm);
> +    }
> +    else {
> +        av_log(avctx, AV_LOG_ERROR, "invalid chunk id: %d\n", chunk_id);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    return avpkt->size;
> +}
> +
> +static av_cold int hnm_decode_init(AVCodecContext *avctx) {
> +
> +    Hnm4VideoContext *hnm = avctx->priv_data;
> +
> +    hnm->buffer1 = av_mallocz(avctx->width * avctx->height);
> +    hnm->buffer2 = av_mallocz(avctx->width * avctx->height);
> +    hnm->processed = av_mallocz(avctx->width * avctx->height);
> +    hnm->palette = av_mallocz(256 * 4);
> +
> +    if(hnm->buffer1 == NULL || hnm->buffer2 == NULL
> +            || hnm->processed == NULL || hnm->palette == NULL) {
> +        av_log(avctx, AV_LOG_ERROR, "av_mallocz() failed\n");
> +        return AVERROR(ENOMEM);

you forget to free already allocated buffers here (i.e. what if allocation
fails on hnm->processed ?)
Also it's silly to allocate palette this way - just declare it as an array of
fixed size in context.

> +    }
> +
> +    hnm->current = hnm->buffer1;
> +    hnm->previous = hnm->buffer2;
> +
> +    avctx->pix_fmt = AV_PIX_FMT_PAL8;
> +    hnm->version = avctx->extradata[0];

you should check for avctx->extradata_size being > 0 here

> +    hnm->width = avctx->width;
> +    hnm->height = avctx->height;
> +
> +    return 0;
> +}
> +
> +static av_cold int hnm_decode_end(AVCodecContext *avctx) {
> +
> +    Hnm4VideoContext *hnm = avctx->priv_data;
> +
> +    av_free(hnm->buffer1);
> +    av_free(hnm->buffer2);
> +    av_free(hnm->processed);
> +    av_free(hnm->palette);

please use av_freep(&hnm->some_buffer) here

> +
> +    return 0;
> +}
> +
> +AVCodec ff_hnm4_video_decoder = {
> +    .name           = "hnm4video",
> +    .long_name      = NULL_IF_CONFIG_SMALL("HNM 4 video"),
> +    .type           = AVMEDIA_TYPE_VIDEO,
> +    .id             = AV_CODEC_ID_HNM4_VIDEO,
> +    .priv_data_size = sizeof(Hnm4VideoContext),
> +    .init           = hnm_decode_init,
> +    .close          = hnm_decode_end,
> +    .decode         = hnm_decode_frame,
> +    .capabilities   = CODEC_CAP_DR1,
> +};
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index f12b493..3ac30eb 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -141,6 +141,7 @@ OBJS-$(CONFIG_H264_DEMUXER)              +=
> h264dec.o rawdec.o
>  OBJS-$(CONFIG_H264_MUXER)                += rawenc.o
>  OBJS-$(CONFIG_HLS_DEMUXER)               += hls.o
>  OBJS-$(CONFIG_HLS_MUXER)                 += hlsenc.o
> +OBJS-$(CONFIG_HNM_DEMUXER)               += hnm.o
>  OBJS-$(CONFIG_IDCIN_DEMUXER)             += idcin.o
>  OBJS-$(CONFIG_IFF_DEMUXER)               += iff.o
>  OBJS-$(CONFIG_ILBC_DEMUXER)              += ilbc.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index 585cf43..289639e 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -118,6 +118,7 @@ void av_register_all(void)
>      REGISTER_MUXDEMUX(H263,             h263);
>      REGISTER_MUXDEMUX(H264,             h264);
>      REGISTER_MUXDEMUX(HLS,              hls);
> +    REGISTER_DEMUXER (HNM,              hnm);
>      REGISTER_DEMUXER (IDCIN,            idcin);
>      REGISTER_DEMUXER (IFF,              iff);
>      REGISTER_MUXDEMUX(ILBC,             ilbc);
> diff --git a/libavformat/hnm.c b/libavformat/hnm.c
> new file mode 100644
> index 0000000..6c80c69
> --- /dev/null
> +++ b/libavformat/hnm.c
> @@ -0,0 +1,203 @@
> +/*
> + * Cryo Interactive Entertainment HNM4 demuxer
> + * Copyright (c) 2012 David Kment
> + *
> + * This file is part of Libav.
> + *
> + * Libav is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * Libav is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with Libav; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +#include "avformat.h"
> +#include "internal.h"
> +#include "libavutil/intreadwrite.h"
> +
> +#define HNM4_TAG MKTAG('H', 'N', 'M', '4')
> +
> +#define HNM4_SAMPLE_RATE 22050
> +#define HNM4_FRAME_FPS 24
> +
> +#define HNM4_CHUNK_ID_PL 19536
> +#define HNM4_CHUNK_ID_IZ 23113
> +#define HNM4_CHUNK_ID_IU 21833
> +#define HNM4_CHUNK_ID_SD 17491
> +
> +typedef struct Hnm4DemuxContext {
> +
> +    uint8_t version;
> +    uint16_t width;
> +    uint16_t height;
> +    uint32_t filesize;
> +    uint32_t frames;
> +    uint32_t taboffset;
> +    uint16_t bits;
> +    uint16_t channels;
> +    uint32_t framesize;
> +    uint32_t currentframe;
> +
> +    int64_t pts;
> +
> +    uint32_t superchunk_remaining;
> +
> +    AVPacket vpkt;
> +
> +} Hnm4DemuxContext;
> +
> +static int hnm_probe(AVProbeData *p)
> +{
> +    if(p->buf_size < 4) {
> +        return 0;
> +    }
> +
> +    // check for HNM4 header
> +    if(AV_RL32(&p->buf[0]) == HNM4_TAG) {
> +        return AVPROBE_SCORE_MAX;
> +    }
> +
> +    return 0;
> +}
> +
> +static int hnm_read_header(AVFormatContext *s)
> +{
> +    Hnm4DemuxContext *hnm = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    AVStream *vst;
> +
> +    /* default context members */
> +    hnm->pts = 0;
> +    av_init_packet(&hnm->vpkt);
> +    hnm->vpkt.data = NULL; hnm->vpkt.size = 0;
> +
> +    hnm->superchunk_remaining = 0;
> +
> +    avio_skip(pb, 8);
> +    hnm->width = avio_rl16(pb);
> +    hnm->height = avio_rl16(pb);
> +    hnm->filesize = avio_rl32(pb);
> +    hnm->frames = avio_rl32(pb);
> +    hnm->taboffset = avio_rl32(pb);
> +    hnm->bits = avio_rl16(pb);
> +    hnm->channels = avio_rl16(pb);
> +    hnm->framesize = avio_rl32(pb);
> +    avio_skip(pb, 32);
> +

please check that you don't read silly values here (e.g 1000000x0 video with
128 channels audio)

> +    hnm->currentframe = 0;
> +
> +    // TODO: find a better way to detect HNM4A
> +    if(hnm->width >= 640) {
> +        hnm->version = 0x4a;
> +    } else {
> +        hnm->version = 0x40;
> +    }
> +
> +    if (!(vst = avformat_new_stream(s, NULL)))
> +        return AVERROR(ENOMEM);
> +
> +    vst->codec->extradata = av_mallocz(1);
> +    vst->codec->extradata_size = 1;
> +    memcpy(vst->codec->extradata, &hnm->version, 1);
> +    vst->codec->codec_type = AVMEDIA_TYPE_VIDEO;
> +    vst->codec->codec_id = AV_CODEC_ID_HNM4_VIDEO;
> +    vst->codec->codec_tag = 0;
> +    vst->codec->width = hnm->width;
> +    vst->codec->height = hnm->height;
> +
> +    vst->start_time = 0;
> +
> +    avpriv_set_pts_info(vst, 33, 1, HNM4_FRAME_FPS);
> +
> +    return 0;
> +}
> +
> +static int hnm_read_packet(AVFormatContext *s,
> +                           AVPacket *pkt)
> +{
> +    Hnm4DemuxContext *hnm = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    int ret = 0;
> +
> +    uint32_t superchunk_size;
> +    uint32_t chunk_size;
> +    uint16_t chunk_id;
> +
> +    if(hnm->currentframe == hnm->frames) {
> +        return AVERROR(EPIPE);
> +    }
> +
> +    if(pb->eof_reached ||  (int) avio_tell(pb) >= hnm->filesize) {
> +        return AVERROR(EPIPE);

we have AVERROR_EOF for these

> +    }
> +
> +    if(hnm->superchunk_remaining == 0) {
> +
> +        /* parse next superchunk */
> +        superchunk_size = avio_rl24(pb);
> +        avio_skip(pb, 1);
> +
> +        hnm->superchunk_remaining = superchunk_size - 4;
> +    }
> +
> +    chunk_size = avio_rl24(pb);
> +    avio_skip(pb, 1);
> +    chunk_id = avio_rl16(pb);
> +    avio_skip(pb, 2);
> +
> +    switch(chunk_id) {
> +
> +        case HNM4_CHUNK_ID_PL:
> +        case HNM4_CHUNK_ID_IZ:
> +        case HNM4_CHUNK_ID_IU:
> +            avio_seek(pb, -8, SEEK_CUR);
> +            ret += av_get_packet(pb, pkt, chunk_size);
> +            hnm->superchunk_remaining -= chunk_size;
> +            if(chunk_id == HNM4_CHUNK_ID_IZ || chunk_id ==
> HNM4_CHUNK_ID_IU) {
> +                hnm->currentframe++;
> +            }
> +            break;
> +
> +        case HNM4_CHUNK_ID_SD:
> +            avio_skip(pb, chunk_size - 8);
> +            hnm->superchunk_remaining -= chunk_size;
> +            break;
> +
> +        default:
> +            av_log(s, AV_LOG_WARNING, "unknown chunk found: %d,
> offset: %d\n", chunk_id, (int) avio_tell(pb));
> +            avio_skip(pb, chunk_size - 8);
> +            hnm->superchunk_remaining -= chunk_size;
> +            break;
> +    }

it would be nice to check chunk sizes too

> +    return ret;
> +}
> +
> +static int hnm_read_close(AVFormatContext *s)
> +{
> +    Hnm4DemuxContext *hnm = s->priv_data;
> +
> +    if (hnm->vpkt.size > 0)
> +        av_free_packet(&hnm->vpkt);
> +
> +    return 0;
> +}
> +
> +AVInputFormat ff_hnm_demuxer = {
> +    .name           = "hnm4movie",
> +    .long_name      = NULL_IF_CONFIG_SMALL("HNM4 movie"),
> +    .priv_data_size = sizeof(Hnm4DemuxContext),
> +    .read_probe     = hnm_probe,
> +    .read_header    = hnm_read_header,
> +    .read_packet    = hnm_read_packet,
> +    .read_close     = hnm_read_close,
> +    .flags          = AVFMT_NO_BYTE_SEEK
> +};

In general it's a very nice work, it just needs to be a little updated to our
current standards.
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to