[FFmpeg-devel] [PATCH] Add ICO muxer

Peter Ross pross at xvid.org
Sun Aug 12 09:57:38 CEST 2012


On Sat, Aug 11, 2012 at 03:15:26PM -0600, Michael Bradshaw wrote:
> On Thu, Aug 9, 2012 at 8:35 PM, Michael Bradshaw
> <mbradshaw at sorensonmedia.com> wrote:
> > Hi,
> >
> > Attached patch adds support for ICO muxing. I expect I'll need to make
> > changes, so please don't hold back in your reviews (I've never written
> > a muxer before so I'm still unsure if I got all the API contracts
> > right). I'm mostly unsure of the flushing (if I didn't flush, my very
> > small files would have missing data).
> 
> Sorry, previous patch had an error. Attached is a revised patch (note
> I'm still unsure of the flushing...).

> From d6a930b264250e94981b66a368d68391eb6c1c77 Mon Sep 17 00:00:00 2001
> From: Michael Bradshaw <mbradshaw at sorensonmedia.com>
> Date: Sat, 11 Aug 2012 15:12:15 -0600
> Subject: [PATCH] Add ICO muxer
> 
> Signed-off-by: Michael Bradshaw <mbradshaw at sorensonmedia.com>
> ---
>  libavformat/Makefile     |    1 +
>  libavformat/allformats.c |    2 +-
>  libavformat/icoenc.c     |  223 ++++++++++++++++++++++++++++++++++++++++++++++


Don't forget to update the documentation (specifically Changelog and doc/general.texi)

>  3 files changed, 225 insertions(+), 1 deletions(-)
>  create mode 100644 libavformat/icoenc.c
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index d77c9ea..d61c990 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -122,6 +122,7 @@ OBJS-$(CONFIG_H264_DEMUXER)              += h264dec.o rawdec.o
>  OBJS-$(CONFIG_H264_MUXER)                += rawenc.o
>  OBJS-$(CONFIG_HLS_DEMUXER)               += hls.o
>  OBJS-$(CONFIG_ICO_DEMUXER)               += icodec.o
> +OBJS-$(CONFIG_ICO_MUXER)                 += icoenc.o
>  OBJS-$(CONFIG_IDCIN_DEMUXER)             += idcin.o
>  OBJS-$(CONFIG_IDF_DEMUXER)               += bintext.o
>  OBJS-$(CONFIG_IFF_DEMUXER)               += iff.o
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index d1b41e6..9df6280 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -112,7 +112,7 @@ void av_register_all(void)
>      REGISTER_MUXDEMUX (H263, h263);
>      REGISTER_MUXDEMUX (H264, h264);
>      REGISTER_DEMUXER  (HLS, hls);
> -    REGISTER_DEMUXER  (ICO, ico);
> +    REGISTER_MUXDEMUX (ICO, ico);
>      REGISTER_DEMUXER  (IDCIN, idcin);
>      REGISTER_DEMUXER  (IDF, idf);
>      REGISTER_DEMUXER  (IFF, iff);
> diff --git a/libavformat/icoenc.c b/libavformat/icoenc.c
> new file mode 100644
> index 0000000..5dc93d4
> --- /dev/null
> +++ b/libavformat/icoenc.c
> @@ -0,0 +1,223 @@
> +/*
> + * Microsoft Windows ICO muxer
> + * Copyright (c) 2012 Michael Bradshaw <mbradshaw at sorensonmedia.com>
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +/**
> + * @file
> + * Microsoft Windows ICO muxer
> + */
> +
> +#include "libavutil/intreadwrite.h"
> +#include "libavutil/pixdesc.h"
> +#include "avformat.h"
> +
> +typedef struct {
> +    int offset;
> +    int size;
> +    unsigned char width;
> +    unsigned char height;
> +    short bits;
> +} IcoImage;
> +
> +typedef struct {
> +    int current_image;
> +    int nb_images;
> +    IcoImage *images;
> +} IcoMuxContext;
> +
> +static int ico_check_attributes(AVFormatContext *s, const AVCodecContext *c)
> +{
> +    // Supported BMP pixel formats in ICO files:
> +    // Depth  Format
> +    // 1bit:  pal8
> +    // 4bit:  pal8
> +    // 8bit:  pal8
> +    // 16bit: rgb555le
> +    // 24bit: bgr24
> +    // 32bit: bgra
> +
> +    // Supported PNG pixel formats in ICO files:
> +    // Depth  Format
> +    // 32bit: rgba

The information above is probably best to be put in general.texi

> +    if (c->codec_id == CODEC_ID_BMP) {
> +        if (c->pix_fmt == PIX_FMT_PAL8 && PIX_FMT_RGB32 != PIX_FMT_BGRA) {
> +            av_log(s, AV_LOG_ERROR, "Error: wrong endianness for bmp pixel format\n");

Dont normally need to prefix things with 'Error: '
This applies to this av_log(), and all others in your patch.

> +            return AVERROR(EINVAL);
> +        } else if (c->pix_fmt != PIX_FMT_PAL8 && // TODO: fix error message to contain the right message
> +                   c->pix_fmt != PIX_FMT_RGB555LE &&
> +                   c->pix_fmt != PIX_FMT_BGR24 &&
> +                   c->pix_fmt != PIX_FMT_BGRA) {
> +            av_log(s, AV_LOG_ERROR, "Error: bmp must be 1bit, 4bit, 8bit, 16bit, 24bit, or 32bit\n");
> +            return AVERROR(EINVAL);
> +        }
> +    } else if (c->codec_id == CODEC_ID_PNG) {
> +        if (c->pix_fmt != PIX_FMT_RGBA) {
> +            av_log(s, AV_LOG_ERROR, "Error: png in ico requires pixel format to be rgba\n");
> +            return AVERROR(EINVAL);
> +        }
> +    } else {
> +        av_log(s, AV_LOG_ERROR, "Error: unsupported codec %s\n", c->codec_name);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    if (c->width > 256 ||
> +        c->height > 256) {
> +        av_log(s, AV_LOG_ERROR, "Error: unsupported dimensions %dx%d (dimensions cannot exceed 256x256)\n", c->width, c->height);
> +        return AVERROR(EINVAL);
> +    }
> +
> +    return 0;
> +}
> +
> +static int ico_write_header(AVFormatContext *s)
> +{
> +    IcoMuxContext *ico = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    int ret;
> +    int i;
> +
> +    if (!pb->seekable) {
> +        av_log(s, AV_LOG_ERROR, "Error: output is not seekable\n");
> +        return AVERROR(EINVAL);
> +    }
> +
> +    ico->current_image = 0;
> +    ico->nb_images = s->nb_streams;
> +
> +    avio_wl16(pb, 0); // reserved
> +    avio_wl16(pb, 1); // 1 == icon
> +    avio_wl16(pb, ico->nb_images);
> +
> +    for (i = 0; i < s->nb_streams; i++) {
> +        if (ret = ico_check_attributes(s, s->streams[i]->codec))
> +            return ret;
> +
> +        // Fill in later when writing trailer...
> +        avio_wl64(pb, 0);
> +        avio_wl64(pb, 0);

avio_skip?

> +    }
> +
> +    ico->images = av_mallocz(ico->nb_images * sizeof(IcoMuxContext));
> +    if (!ico->images)
> +        return AVERROR(ENOMEM);
> +
> +    avio_flush(pb);
> +
> +    return 0;
> +}
> +
> +static int ico_write_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    IcoMuxContext *ico = s->priv_data;
> +    IcoImage *image;
> +    AVIOContext *pb = s->pb;
> +    AVCodecContext *c = s->streams[pkt->stream_index]->codec;
> +    int i;
> +
> +    if (ico->current_image >= ico->nb_images) {
> +        av_log(s, AV_LOG_ERROR, "Error: ico already contains %d images\n", ico->current_image);
> +        return AVERROR(EIO);
> +    }
> +
> +    image = &ico->images[ico->current_image++];
> +
> +    image->offset = avio_tell(pb);
> +    image->width = (c->width == 256) ? 0 : c->width;
> +    image->height = (c->height == 256) ? 0 : c->height;
> +
> +    if (c->codec_id == CODEC_ID_PNG) {
> +        image->bits = c->bits_per_coded_sample;
> +        image->size = pkt->size;
> +
> +        avio_write(pb, pkt->data, pkt->size);
> +    } else { // BMP
> +        if (AV_RL32(pkt->data + 14) != 40) { // must be BITMAPINFOHEADER
> +            av_log(s, AV_LOG_ERROR, "Error: invalid bmp\n");
> +            return AVERROR(EINVAL);
> +        }
> +
> +        image->bits = AV_RL16(pkt->data + 28); // allows things like 1bit and 4bit images to be preserved
> +        image->size = pkt->size - 14 + c->height * (c->width + 7) / 8;
> +
> +        avio_write(pb, pkt->data + 14, 8); // Skip the BITMAPFILEHEADER header
> +        avio_wl32(pb, AV_RL32(pkt->data + 22) * 2); // rewrite height as 2 * height
> +        avio_write(pb, pkt->data + 26, pkt->size - 26);
> +
> +        for (i = 0; i < c->height * (c->width + 7) / 8; ++i)
> +            avio_w8(pb, 0x00); // Write bitmask (opaque)
> +    }
> +
> +    avio_flush(pb);
> +
> +    return 0;
> +}
> +
> +static int ico_write_trailer(AVFormatContext *s)
> +{
> +    IcoMuxContext *ico = s->priv_data;
> +    AVIOContext *pb = s->pb;
> +    int i;
> +
> +    if (ico->current_image != ico->nb_images) {
> +        av_log(s, AV_LOG_ERROR, "Error: ico is missing %d frames\n", ico->nb_images - ico->current_image);
> +        return AVERROR(EIO);

This situation will occur when the user interrupts the muxer. It would be nice if the muxer would
generate a working .ico file when this happens.  e.g. defer 'avio_wl16(pb, ico->nb_images)' until
ico_write_trailer.

> +    }
> +
> +    avio_seek(pb, 6, SEEK_SET);
> +
> +    for (i = 0; i < ico->nb_images; i++) {
> +        avio_w8(pb, ico->images[i].width);
> +        avio_w8(pb, ico->images[i].height);
> +
> +        if (s->streams[i]->codec->codec_id == CODEC_ID_BMP &&
> +            s->streams[i]->codec->pix_fmt == PIX_FMT_PAL8) {
> +            avio_w8(pb, (ico->images[i].bits >= 8) ? 0 : 1 << ico->images[i].bits);
> +        } else {
> +            avio_w8(pb, 0);
> +        }
> +
> +        avio_w8(pb, 0); // reserved
> +        avio_wl16(pb, 1); // color planes
> +        avio_wl16(pb, ico->images[i].bits);
> +        avio_wl32(pb, ico->images[i].size);
> +        avio_wl32(pb, ico->images[i].offset);
> +    }
> +
> +    avio_flush(pb);
> +
> +    av_freep(&ico->images);
> +
> +    return 0;
> +}
> +
> +AVOutputFormat ff_ico_muxer = {
> +    .name           = "ico",
> +    .long_name      = NULL_IF_CONFIG_SMALL("Microsoft Windows ICO"),
> +    .priv_data_size = sizeof(IcoMuxContext),
> +    .mime_type      = "image/vnd.microsoft.icon",
> +    .extensions     = "ico",
> +    .audio_codec    = CODEC_ID_NONE,
> +    .video_codec    = CODEC_ID_BMP,
> +    .write_header   = ico_write_header,
> +    .write_packet   = ico_write_packet,
> +    .write_trailer  = ico_write_trailer,
> +    .flags          = AVFMT_NOTIMESTAMPS,
> +};
> -- 
> 1.7.7

I can't really help you with the flushing problem. I thought this is only required
if you want a partially muxed file to be usable by another process. And, in the case
of ico, that's not possible, because the image offsets are written at the ico_write_trailer.

Cheers,

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


More information about the ffmpeg-devel mailing list