On 02/14/2012 03:54 PM, Aneesh Dogra wrote:
> ---
> Changelog | 1 +
> libavcodec/Makefile | 1 +
> libavcodec/allcodecs.c | 2 +-
> libavcodec/sunrastenc.c | 246
> +++++++++++++++++++++++++++++++++++++++++++++++
> libavformat/img2.c | 3 +-
> 5 files changed, 251 insertions(+), 2 deletions(-)
> create mode 100644 libavcodec/sunrastenc.c
You're missing an update to doc/general.texi
>
> diff --git a/Changelog b/Changelog
> index cc7420c..412520e 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,7 @@ version <next>:
> - XWD encoder and decoder
> - Support for fragmentation in the mov/mp4 muxer
> - ISMV (Smooth Streaming) muxer
> +- Sun Rasterfile Encoder
>
>
> version 0.8:
> diff --git a/libavcodec/Makefile b/libavcodec/Makefile
> index a891651..88faefb 100644
> --- a/libavcodec/Makefile
> +++ b/libavcodec/Makefile
> @@ -354,6 +354,7 @@ OBJS-$(CONFIG_SOL_DPCM_DECODER) += dpcm.o
> OBJS-$(CONFIG_SP5X_DECODER) += sp5xdec.o mjpegdec.o mjpeg.o
> OBJS-$(CONFIG_SRT_DECODER) += srtdec.o ass.o
> OBJS-$(CONFIG_SUNRAST_DECODER) += sunrast.o
> +OBJS-$(CONFIG_SUNRAST_ENCODER) += sunrastenc.o
> OBJS-$(CONFIG_SVQ1_DECODER) += svq1dec.o svq1.o h263.o \
> mpegvideo.o error_resilience.o
> OBJS-$(CONFIG_SVQ1_ENCODER) += svq1enc.o svq1.o \
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index cda71e0..69a37b2 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -188,7 +188,7 @@ void avcodec_register_all(void)
> REGISTER_DECODER (SMC, smc);
> REGISTER_ENCDEC (SNOW, snow);
> REGISTER_DECODER (SP5X, sp5x);
> - REGISTER_DECODER (SUNRAST, sunrast);
> + REGISTER_ENCDEC (SUNRAST, sunrast);
> REGISTER_ENCDEC (SVQ1, svq1);
> REGISTER_DECODER (SVQ3, svq3);
> REGISTER_ENCDEC (TARGA, targa);
> diff --git a/libavcodec/sunrastenc.c b/libavcodec/sunrastenc.c
> new file mode 100644
> index 0000000..84fcc51
> --- /dev/null
> +++ b/libavcodec/sunrastenc.c
> @@ -0,0 +1,246 @@
> +/*
> + * Sun Rasterfile (.sun/.ras/im{1,8,24}/.sunras) image encoder
> + * Copyright (c) 2012 Aneesh Dogra (lionaneesh) <[email protected]>
> + *
> + * 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 "bytestream.h"
> +#include "internal.h"
> +#include "libavutil/pixdesc.h"
> +
> +#define RAS_MAGIC 0x59a66a95
> +
> +/* Both Old and Standard formats are the same.
> + * They indicate that the image data within the file is not compressed */
> +#define RT_OLD 0
> +#define RT_STANDARD 1
> +
> +/* The Byte-encoded type indicates that the image data is compressed using a
> + * Run-length encoding scheme */
> +#define RT_BYTE_ENCODED 2
> +#define RLE_TRIGGER 0x80
> +
> +#define RT_FORMAT_RGB 3
> +
> +/* The TIFF and IFF format types indicate that the raster file was originally
> + * converted from either of these file formats. */
> +#define RT_FORMAT_TIFF 4
> +#define RT_FORMAT_IFF 5
> +
> +/* The Experimental type is implementation-specific and is generally an
> + indication that the image file does not conform to the Sun Raster file
> + format specification */
> +#define RT_EXPERIMENTAL 0xffff
> +
> +#define RMT_NONE 0
> +#define RMT_EQUAL_RGB 1
> +#define RMT_RAW 2
most of this stuff looks duplicated from the decoder. please move them
to a shared header.
> +
> +typedef struct SUNRASTContext {
> + AVFrame picture;
> + PutByteContext p;
> + int depth; // depth (1, 8, or 24 bits) of pixel
> + int length; // length (bytes) of image
> + int type; // type of file
> + int maptype; // type of colormap
> + int maplength; // length (bytes) of following map
watch the tabs. and use doxygen-style comments.
> + int size;
> +} SUNRASTContext;
> +
> +static int sunrast_image_write_header(AVCodecContext *avctx,
> + SUNRASTContext *s)
> +{
> + bytestream2_put_be32(&s->p, RAS_MAGIC);
> + bytestream2_put_be32(&s->p, avctx->width);
> + bytestream2_put_be32(&s->p, avctx->height);
> + bytestream2_put_be32(&s->p, s->depth);
> + bytestream2_put_be32(&s->p, s->length);
> + bytestream2_put_be32(&s->p, s->type);
> + bytestream2_put_be32(&s->p, s->maptype);
> + bytestream2_put_be32(&s->p, s->maplength);
here, and elsewhere, you can use the unchecked writer since you know the
output packet will always be large enough.
> +
> + return 0;
make this a void function since it only ever returns 0.
> +}
> +
> +static int sunrast_image_write_image(AVCodecContext *avctx, SUNRASTContext
> *s,
> + const uint8_t *pixels,
> + const uint32_t *palette_data,
> + int linesize)
> +{
> + const uint8_t *ptr;
> + uint32_t pixel;
> + int x, y, len, alen, r, g, b;
> + PutByteContext pb_r, pb_g;
> +
> + if (s->maplength) { // palettized
> + unsigned int len = s->maplength / 3;
> +
> + pb_r = s->p; bytestream2_skip_p(&s->p, len);
> + pb_g = s->p; bytestream2_skip_p(&s->p, len);
This looks wrong. You have pb_r and pb_g pointing to the same place.
Also, keep one statement per line please.
> +
> + for (x = 0; x < len; x++) {
> + pixel = palette_data[x];
> + r = (pixel >> 16) & 0xFF;
> + g = (pixel >> 8) & 0xFF;
> + b = pixel & 0xFF;
> +
> + bytestream2_put_byte(&pb_r, r);
> + bytestream2_put_byte(&pb_g, g);
> + bytestream2_put_byte(&s->p, b);
> + }
> +
> + }
remove the extra blank line.
> +
> + len = (s->depth * avctx->width + 7) >> 3;
> + alen = len + (len & 1);
> + ptr = pixels;
> +
> + if (s->type == RT_BYTE_ENCODED) {
> + uint8_t value, value2;
> + int run;
> + const uint8_t *end = pixels + avctx->height * linesize;
> + int x = 0;
> +
> + ptr = pixels;
> +#define GET_VALUE (ptr >= end || x > len) ? 0 : x == len ? ptr[len-1] :
> ptr[x]
this can be:
(ptr >= end || x >= len) ? ptr[len-1] : ptr[x]
> +
> + while (ptr < end) {
> + run = 1;
> + value = GET_VALUE;
> + x++;
> + if (x >= alen) {
> + x = 0;
> + ptr += linesize;
> + }
> +
> + value2 = GET_VALUE;
> + while (value2 == value && run < 255 && ptr < end) {
run can be 256.
> + x++;
> + run++;
> + if (x >= alen) {
> + x = 0;
> + ptr += linesize;
> + }
> + value2 = GET_VALUE;
> + }
> +
> + if (run == 1 && value != RLE_TRIGGER) {
> + bytestream2_put_byte(&s->p, value);
> + }
> + else {
} else {
> + bytestream2_put_byte(&s->p, RLE_TRIGGER);
> + bytestream2_put_byte(&s->p, run - 1);
> + if (run > 1)
> + bytestream2_put_byte(&s->p, value);
> + }
> + }
> + } else {
> + for (y = 0; y < avctx->height; y++) {
> + bytestream2_put_buffer(&s->p, ptr, len);
> + bytestream2_seek_p(&s->p, len & 1, SEEK_CUR);
seeking leaves the output undefined. write a 0 byte instead.
> + ptr += linesize;
> + }
> + }
> +
> + return 0;
does this return anything other than 0? if not, just make it a void
function.
> +}
> +
> +static av_cold int sunrast_encode_init(AVCodecContext *avctx)
> +{
> + SUNRASTContext *s = avctx->priv_data;
> + int pixels = avctx->width * avctx->height;
> +
> + avctx->coded_frame = &s->picture;
> +
> + s->maplength = 0;
> + s->type = (avctx->coder_type == FF_CODER_TYPE_RLE) ?
> + RT_BYTE_ENCODED : RT_STANDARD;
> + s->maptype = RMT_NONE;
> +
> + switch (avctx->pix_fmt) {
> + case PIX_FMT_MONOWHITE: // Monochrome
case should line up vertically with switch.
also, the pixel format should be enough info, no need to document that
it's monochrome. same for the other pixel formats too.
> + s->depth = 1;
> + s->length = ((avctx->width + 7) >> 3) * avctx->height +
> + (((avctx->width / 8) + (avctx->width % 8 ? 1 : 0)) %
> 2 ?
> + avctx->height : 0);
that's quite a formula... any way to simplify this?
> + break;
> + case PIX_FMT_PAL8 :
> + case PIX_FMT_GRAY8:
> + s->depth = 8;
> + s->length = pixels + ((unsigned int)(avctx->width & 0x01) ?
> + avctx->height : 0);
no need to cast to unsigned int
> + if (avctx->pix_fmt == PIX_FMT_PAL8) { // With palette
> + s->maptype = RMT_EQUAL_RGB;
> + s->maplength = 3 * 256;
> + }
> + break;
> + case PIX_FMT_BGR24: // Full Color
> + s->depth = 24;
> + s->length = 3 * pixels + ((unsigned int)(avctx->width & 0x01) ?
> + avctx->height :
> 0);
same here
> + break;
> + default:
> + av_log(avctx, AV_LOG_ERROR, "invalid pixel format\n");
> + return AVERROR(EINVAL);
this is validated in avcodec_open() so you don't have to check it here.
maybe just return AVERROR_BUG. no need for a log message though.
> + }
> +
> + s->size = 32 + s->maplength + s->length;
> +
> + return 0;
> +}
> +
> +static int sunrast_encode_frame(AVCodecContext *avctx, AVPacket *avpkt,
> + const AVFrame *frame, int *got_packet_ptr)
> +{
> + SUNRASTContext *s = avctx->priv_data;
> + AVFrame *const p = (AVFrame *)&s->picture;
> + int ret;
> +
> + if ((ret = ff_alloc_packet(avpkt, s->size *
> + ((s->type == RT_BYTE_ENCODED) ? 2 : 1))) < 0)
> + return ret;
as far as i can see, s->size isn't used for anything else, so you can
multiply by 2 if needed in init()
> +
> + bytestream2_init_writer(&s->p, avpkt->data, avpkt->size);
> + *p = *frame;
> + p->pict_type = AV_PICTURE_TYPE_I;
set pict_type in init()
> + p->key_frame = 1;
> + sunrast_image_write_header(avctx, s);
> + sunrast_image_write_image(avctx, s, frame->data[0],
No need to pass both avctx and s. You can put an avctx pointer in your
SUNRASTContext if you need to access avctx fields.
> + (const uint32_t *)frame->data[1],
> + frame->linesize[0]);
for RLE you need to go back and rewrite the header with a new 'length'
value.
> +
> + *got_packet_ptr = 1;
> + avpkt->size = bytestream2_tell_p(&s->p);
> + return bytestream2_tell_p(&s->p);
return 0;
> +}
> +
> +AVCodec ff_sunrast_encoder = {
> + .name = "sunrast",
> + .type = AVMEDIA_TYPE_VIDEO,
> + .id = CODEC_ID_SUNRAST,
> + .priv_data_size = sizeof(SUNRASTContext),
> + .init = sunrast_encode_init,
> + .encode2 = sunrast_encode_frame,
> + .capabilities = CODEC_CAP_DR1,
using DR1 doesn't make sense for an encoder
> + .pix_fmts = (const enum PixelFormat[]){ PIX_FMT_BGR24,
> + PIX_FMT_PAL8,
> + PIX_FMT_GRAY8,
> + PIX_FMT_MONOWHITE },
> + .long_name = NULL_IF_CONFIG_SMALL("Sun Rasterfile image Encoder"),
leave off the "Encoder". it should match the string int the decoder.
> +};
> diff --git a/libavformat/img2.c b/libavformat/img2.c
> index f7a9fa5..d7a749c 100644
> --- a/libavformat/img2.c
> +++ b/libavformat/img2.c
> @@ -491,7 +491,8 @@ AVOutputFormat ff_image2_muxer = {
> .name = "image2",
> .long_name = NULL_IF_CONFIG_SMALL("image2 sequence"),
> .extensions = "bmp,dpx,jpeg,jpg,ljpg,pam,pbm,pcx,pgm,pgmyuv,png,"
> - "ppm,sgi,tga,tif,tiff,jp2,xwd",
> + "ppm,sgi,tga,tif,tiff,jp2,xwd,sun,ras,rs,im1,im8,im24,"
> + "sunras",
all those extensions are really commonly used for the same format?
> .priv_data_size = sizeof(VideoData),
> .video_codec = CODEC_ID_MJPEG,
> .write_header = write_header,
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
