[FFmpeg-devel] [PATCH] IFF: Add the HAM stuff
Sebastian Vater
cdgs.basty
Sun May 16 00:40:21 CEST 2010
Stefano Sabatini a ?crit :
> On date Saturday 2010-05-15 23:12:25 +0200, Sebastian Vater encoded:
>
>
>> diff --git a/libavcodec/iff.c b/libavcodec/iff.c
>> index 3cdc964..0ff81be 100644
>> --- a/libavcodec/iff.c
>> +++ b/libavcodec/iff.c
>> @@ -30,10 +30,37 @@
>> #include "get_bits.h"
>> #include "iff.h"
>>
>> +// TODO: masking bits
>> +#define MASK_NONE 0
>> +#define MASK_HAS_MASK 1
>> +#define MASK_HAS_TRANSPARENT_COLOR 2
>> +#define MASK_LASSO 3
>> +
>> +/**
>> + * Gets the actual packet data after video properties which contains
>> + * the raw data beyond the IFF extra context.
>> + */
>> +#define GET_EXTRA_DATA(x) ((uint8_t *) (x)->data + AV_RB16((x)->data))
>> +
>> +/**
>> + * Gets the size of raw data beyond the IFF extra context. Please note
>> + * that any value < 2 of either IFF extra context or raw packet data
>> + * is considered as an illegal packet.
>> + */
>> +#define GET_EXTRA_SIZE(x) ((x)->size - AV_RB16((x)->data))
>> +
>> typedef struct {
>> AVFrame frame;
>> int planesize;
>> uint8_t * planebuf;
>> + uint8_t * ham_buf; ///< Temporary buffer for planar to chunky conversation
>> + uint32_t *ham_palbuf; ///< HAM decode table
>> + unsigned compression; ///< Delta compression method used
>> + unsigned bpp; ///< Bits per plane to decode
>> + unsigned ham; ///< 0 if non-HAM or number of hold bits (6 for bpp > 6, 4 otherwise)
>> + unsigned flags; ///< 1 for EHB, 0 is no extra half darkening
>> + unsigned masking; ///< TODO: Masking method used
>> + unsigned transparency; ///< TODO: Transparency color index in palette
>> } IffContext;
>>
>> #define LUT8_PART(plane, v) \
>> @@ -219,6 +246,125 @@ static void decodeplane32(uint32_t *dst, const uint8_t *buf, int buf_size, int p
>> } while (--buf_size);
>> }
>>
>> +#define DECODE_HAM_PLANE32(x) \
>> + first = buf[x] << 1; \
>> + second = buf[(x)+1] << 1; \
>> + delta &= pal[first++]; \
>> + delta |= pal[first]; \
>> + dst[x] = delta; \
>> + delta &= pal[second++]; \
>> + delta |= pal[second]; \
>> + dst[(x)+1] = delta;
>> +
>> +/**
>> + * Converts one line of HAM6/8-encoded chunky buffer to 24bpp
>>
>
> missing final dot.
>
Fixed.
>
>> + *
>> + * @param dst the destination 24bpp buffer
>> + * @param buf the source 8bpp chunky buffer
>> + * @param pal the HAM decode table
>> + * @param buf_size the plane size in bytes
>> + */
>> +static void decode_ham_plane32(uint32_t *dst, const uint8_t *buf,
>> + const uint32_t *const pal, unsigned buf_size)
>> +{
>> + uint32_t delta = 0;
>> + do {
>> + uint32_t first, second;
>> + DECODE_HAM_PLANE32(0)
>> + DECODE_HAM_PLANE32(2)
>> + DECODE_HAM_PLANE32(4)
>> + DECODE_HAM_PLANE32(6)
>> + buf += 8;
>> + dst += 8;
>> + } while (--buf_size);
>> +}
>> +
>> +/**
>> + * Extracts the IFF extra context and updates internal
>> + * decoder structures.
>> + *
>> + * @param avctx the AVCodecContext where to extract extra context to
>> + * @param avpkt the AVPacket to extract extra context from
>> + *
>>
>
> Nit+++: no need for this empty newline
>
Which empty new line? They're just cosmetics like in the other
functions, too. So you see return value separated to args.
>
>> + * @return returns error status. 0 is OK, error otherwise
>>
>
> @return 0 in case of success, a negative error code otherwise
>
Fixed.
>
>> + */
>> +static int extract_header(AVCodecContext *const avctx,
>> + const AVPacket *const avpkt) {
>> + IffContext *s = avctx->priv_data;
>> + const uint8_t *buf = avpkt->data;
>> + const unsigned buf_size = bytestream_get_be16(&buf);
>> + if (buf_size <= 1 || (int) GET_EXTRA_SIZE(avpkt) <= 1) {
>> + av_log(avctx, AV_LOG_ERROR,
>> + "Invalid packet size received: %u -> video data offset: %d\n",
>> + (unsigned) buf_size, (int) GET_EXTRA_SIZE(avpkt));
>> + return AVERROR_INVALIDDATA;
>> + }
>> + if (buf_size > 8) {
>> + s->compression = bytestream_get_byte(&buf);
>> + s->bpp = bytestream_get_byte(&buf);
>> + s->ham = bytestream_get_byte(&buf);
>> + s->flags = bytestream_get_byte(&buf);
>> + s->transparency = bytestream_get_be16(&buf);
>> + s->masking = bytestream_get_byte(&buf);
>> + }
>> +
>> + if (s->masking == MASK_HAS_TRANSPARENT_COLOR) {
>> + av_log(avctx, AV_LOG_ERROR, "Transparency not supported\n");
>> + return AVERROR_PATCHWELCOME;
>> + } else if (s->masking != MASK_NONE) {
>> + av_log(avctx, AV_LOG_ERROR, "Masking not supported\n");
>> + return AVERROR_PATCHWELCOME;
>> + }
>> +
>> + av_freep(&s->ham_buf);
>> + av_freep(&s->ham_palbuf);
>> +
>> + if (s->ham) {
>> + int i;
>> + int count = FFMIN(avctx->extradata_size / 3, 1 << s->ham);
>> + const unsigned mbits = 8 - s->ham;
>> + s->ham_buf = av_malloc((s->planesize * 8) + FF_INPUT_BUFFER_PADDING_SIZE);
>> + if (!s->ham_buf)
>> + return AVERROR(ENOMEM);
>> +
>> + s->ham_palbuf = av_malloc((8 * (1 << s->ham) * sizeof (uint32_t)) + FF_INPUT_BUFFER_PADDING_SIZE);
>> + if (!s->ham_palbuf) {
>> + av_freep(&s->ham_buf);
>> + return AVERROR(ENOMEM);
>> + }
>> +
>> + if (count) { // HAM with color palette attached
>>
>
>
>> + // prefill with black and palette and set take direct color mask to zero
>>
>
> "set take" sounds weird
>
Fixed.
>
>> /**
>> * Decodes one complete byterun1 encoded line.
>> *
>> @@ -256,31 +402,43 @@ static int decode_frame_ilbm(AVCodecContext *avctx,
>> AVPacket *avpkt)
>> {
>> IffContext *s = avctx->priv_data;
>> - const uint8_t *buf = avpkt->data;
>> - int buf_size = avpkt->size;
>> + const uint8_t *buf = GET_EXTRA_DATA(avpkt);
>> + int buf_size = GET_EXTRA_SIZE(avpkt);
>> const uint8_t *buf_end = buf+buf_size;
>> - int y, plane;
>> + int y, plane, err;
>>
>> if (avctx->reget_buffer(avctx, &s->frame) < 0){
>>
>
> Nit++: missing space after closing ")"
>
Will do in a separate patch, since I'm not changing this line.
>
>> av_log(avctx, AV_LOG_ERROR, "get_buffer() failed\n");
>> return -1;
>>
>
> if ((err = ...) < 0) {
> av_log(...)
> return err;
> }
>
Fixed.
>
>> }
>> @@ -292,6 +450,13 @@ static int decode_frame_ilbm(AVCodecContext *avctx,
>> memcpy(row, buf, FFMIN(avctx->width, buf_end - buf));
>> buf += avctx->width;
>> }
>> + } else { // IFF-PBM: HAM to PIX_FMT_BGR32
>> + for(y = 0; y < avctx->height; y++ ) {
>>
>
> for_(
>
Fixed.
>
>> + } else if (s->ham) { // HAM to PIX_FMT_BGR32
>> + for(y = 0; y < avctx->height ; y++ ) {
>> + uint8_t *row = &s->frame.data[0][y*s->frame.linesize[0]];
>> + memset(s->ham_buf, 0, avctx->width);
>> + for (plane = 0; plane < s->bpp; plane++) {
>> + buf = decode_byterun(s->planebuf, buf, buf_end, s->planesize);
>> + decodeplane8(s->ham_buf, s->planebuf, s->planesize, plane);
>> + }
>> + decode_ham_plane32((uint32_t *) row, s->ham_buf, s->ham_palbuf, s->planesize);
>> + }
>> } else { //PIX_FMT_BGR32
>> for(y = 0; y < avctx->height ; y++ ) {
>> uint8_t *row = &s->frame.data[0][y*s->frame.linesize[0]];
>> memset(row, 0, avctx->width << 2);
>> - for (plane = 0; plane < avctx->bits_per_coded_sample; plane++) {
>> + for (plane = 0; plane < s->bpp; plane++) {
>> buf = decode_byterun(s->planebuf, buf, buf_end, s->planesize);
>> decodeplane32((uint32_t *) row, s->planebuf, s->planesize, plane);
>> }
>> }
>> }
>>
>
> Note, this can be factorized (but not in this patch of course ;-)
>
Okay, will do this when the HAM patch is in.
>
>> - } else {
>> + } else if (avctx->pix_fmt == PIX_FMT_PAL8 || avctx->pix_fmt == PIX_FMT_GRAY8) { // IFF-PBM
>> for(y = 0; y < avctx->height ; y++ ) {
>> uint8_t *row = &s->frame.data[0][y*s->frame.linesize[0]];
>> buf = decode_byterun(row, buf, buf_end, avctx->width);
>> }
>> + } else { // IFF-PBM: HAM to PIX_FMT_BGR32
>> + for(y = 0; y < avctx->height ; y++ ) {
>>
>
> Nit++: for_(, no space after y++
>
Fixed.
>
>> +/**
>> + * IFF extra video context. This data is prepended to actual
>> + * video pixel data, always stored in big-endian format.
>> + *
>> + * If you add new data here, always check check whether the packet you
>> + * receive is large enough (offset >= is actually large enough).
>> + *
>> + * uint16_t offset; ///< Offset to add to get raw video pixel data
>> + * uint8_t compression; ///< Delta compression method used
>> + * uint8_t bpp; ///< Bits per plane to decode
>> + * uint8_t ham; ///< 0 if non-HAM or number of hold bits (6 for bpp > 6, 4 otherwise)
>> + * uint8_t flags; ///< TODO: 1 for EHB, 0 is no extra half darkening
>> + * uint16_t transparency; ///< TODO: Transparency color index in palette
>> + * uint8_t masking; ///< TODO: Masking method used
>> + */
>>
>
> This doxy looks misplaced, seems like IffDemuxContext though but I see
> the types are different (maybe having uintX_t is preferable than
> using always unsigned).
>
Fixed.
>
>>
>> +// screenmode bits
>> +#define CAMG_EHB 0x80 // Extra HalfBrite
>> +#define CAMG_HAM 0x800 // Hold And Modify
>> +
>> +/**
>> + * This number of bytes if added at the beginning of each AVPacket
>> + * which contain additional information about video properties
>> + * which has to be shared between demuxer and decoder also changeable
>> + * between frames.
>>
>
> "... and decoder. This number may change between frames."
>
> looks more readable.
>
Fixed.
>
>> + */
>> +#define IFF_EXTRA_VIDEO_SIZE 9
>> +
>> typedef enum {
>> COMP_NONE,
>> COMP_FIB,
>> @@ -77,6 +91,12 @@ typedef struct {
>> uint32_t body_size;
>> uint32_t sent_bytes;
>> uint32_t audio_frame_count;
>> + unsigned compression; ///< Delta compression method used
>> + unsigned bpp; ///< Bits per plane to decode
>> + unsigned ham; ///< 0 if non-HAM or number of hold bits (6 for bpp > 6, 4 otherwise)
>> + unsigned flags; ///< 1 for EHB, 0 is no extra half darkening
>> + unsigned masking; ///< Masking method used
>> + unsigned transparency; ///< Transparency color index in palette
>>
>
> Nit++: no need for capitalization of the field doxies, as they are
> non-complete sentences.
>
Fixed.
>
>> } IffDemuxContext;
>>
>>
>> @@ -129,6 +149,9 @@ static int iff_read_header(AVFormatContext *s,
>> AVStream *st;
>> uint32_t chunk_id, data_size;
>> int compression = -1;
>> + uint32_t screenmode = 0;
>> + unsigned transparency = 0;
>> + unsigned masking = 0; // MASK_NONE
>>
>
> directly use the macro value
>
Fixed.
>>
>> case AVMEDIA_TYPE_VIDEO:
>> + iff->compression = compression;
>> + iff->bpp = st->codec->bits_per_coded_sample;
>> + if ((screenmode & CAMG_HAM) &&
>>
>
>
>> + st->codec->bits_per_coded_sample <= 8) {
>> + iff->ham = st->codec->bits_per_coded_sample > 6 ? 6 : 4;
>>
>
> iff->bpp is shorter.
>
Fixed.
>
>> + iff->masking = masking;
>> + iff->transparency = transparency;
>> +
>> switch (compression) {
>> case BITMAP_RAW:
>> st->codec->codec_id = CODEC_ID_IFF_ILBM;
>> @@ -294,7 +338,21 @@ static int iff_read_packet(AVFormatContext *s,
>> }
>> interleave_stereo(sample_buffer, pkt->data, PACKET_SIZE);
>> } else if (st->codec->codec_type == AVMEDIA_TYPE_VIDEO) {
>> - ret = av_get_packet(pb, pkt, iff->body_size);
>> + uint8_t *buf;
>> +
>> + if(av_new_packet(pkt, iff->body_size + IFF_EXTRA_VIDEO_SIZE) < 0) {
>>
>
> if_(
>
Fixed.
>
>> + return AVERROR(ENOMEM);
>> + }
>> +
>> + buf = pkt->data;
>> + bytestream_put_be16(&buf, IFF_EXTRA_VIDEO_SIZE);
>> + bytestream_put_byte(&buf, iff->compression);
>> + bytestream_put_byte(&buf, iff->bpp);
>> + bytestream_put_byte(&buf, iff->ham);
>> + bytestream_put_byte(&buf, iff->flags);
>> + bytestream_put_be16(&buf, iff->transparency);
>> + bytestream_put_byte(&buf, iff->masking);
>> + ret = get_buffer(pb, buf, iff->body_size);
>> } else {
>> ret = av_get_packet(pb, pkt, PACKET_SIZE);
>> }
>>
--
Best regards,
:-) Basty/CDGS (-:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: iff-ham-support.patch
Type: text/x-patch
Size: 18709 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100516/d3255f5d/attachment.bin>
More information about the ffmpeg-devel
mailing list