[FFmpeg-devel] [PATCH]Add Dirac support to ffmpeg via libdirac_* and Schroedinger libraries]

[Diego Biurrun]

On Thu, Apr 10, 2008 at 05:56:21PM +1000, Anuradha Suraparaju wrote:
> 
> I've attached the dirac patch to this email. It does not include
> dirac_parser.c as the GSoC code can be used without any changes. I've
> created the patch using the latest svn revision 12780. I'll be mailing
> the rest of the patches as soon as possible. 

This patch still contains bits about the parser that do not belong in
there.  If you wish to speed up the review, you could separate the
encoder from the decoder and submit the decoder first.

> --- ffmpegsvn_trunk/configure	2008-04-10 11:02:54.000000000 +1000
> +++ ffmpegsvn_trunk_dirac/configure	2008-04-10 16:25:14.000000000 +1000
> @@ -90,6 +90,8 @@
>    echo "  --enable-libdc1394       enable IIDC-1394 grabbing using libdc1394"
>    echo "                           and libraw1394 [default=no]"
> +  echo "  --enable-libdirac        enable dirac codec support via "
> +  echo "                           libdirac_encoder and libdirac_decoder [default=no]"

Dirac

Also, I would just say "enable Dirac support via libdirac [default=no]".

> @@ -628,6 +630,7 @@
>      beos_netserver
> +    libdirac
>      ffmpeg

Here and everywhere in the build system: Keep things in alphabetical
order.

> @@ -845,6 +850,7 @@
>  libdc1394_demuxer_deps="libdc1394"
> +dirac_demuxer_deps="dirac_parser"

This patch does not add the Dirac parser, so it should not add code
relevant to the Dirac parser, similar issues below.

> --- ffmpegsvn_trunk/libavcodec/libdiracdec.c	1970-01-01 10:00:00.000000000 +1000
> +++ ffmpegsvn_trunk_dirac/libavcodec/libdiracdec.c	2008-04-10 12:32:11.000000000 +1000
> @@ -0,0 +1,190 @@
> +/*
> + * Dirac decoder support via Dirac libdirac_decoder library

s/libdirac_decoder/libdirac/

> +/**
> +* @file libdiracdec.c
> +* Dirac decoder support via Dirac libdirac_decoder library. More
> +* details about the Dirac project can be found at http://dirac.sourceforge.net

nit: URLs should have trailing slashes, same below.  Yes, this is unimportant.

> +    int num_formats = sizeof(ffmpeg_dirac_pixel_format_map)/sizeof(ffmpeg_dirac_pixel_format_map[0]);

Breaking long lines such as this one would be a plus.

> +static int libdirac_decode_frame(AVCodecContext *avccontext,
> +                              void *data, int *data_size,
> +                              uint8_t *buf, int buf_size)

This could be aligned nicely, same in other places.

> --- ffmpegsvn_trunk/libavcodec/libdiracenc.c	1970-01-01 10:00:00.000000000 +1000
> +++ ffmpegsvn_trunk_dirac/libavcodec/libdiracenc.c	2008-04-10 14:57:37.000000000 +1000
> @@ -0,0 +1,395 @@
> +/*
> + * Dirac encoding support via Dirac libdirac_encoder library.

see above

> --- ffmpegsvn_trunk/libavcodec/libdirac.h	1970-01-01 10:00:00.000000000 +1000
> +++ ffmpegsvn_trunk_dirac/libavcodec/libdirac.h	2008-04-09 16:42:53.000000000 +1000
> @@ -0,0 +1,41 @@
> +
> +/**
> +* @file libdirac.h
> +* Data structures common to libdiracenc.c and libdiracdec.c

data

> +static const struct {
> +	enum PixelFormat ff_pix_fmt;
> +	dirac_chroma_t dirac_pix_fmt;
> +} ffmpeg_dirac_pixel_format_map[] = {
> +	{ PIX_FMT_YUV420P, format420 },
> +    { PIX_FMT_YUV422P, format422 },
> +    { PIX_FMT_YUV444P, format444 },
> +};

tabs

> --- ffmpegsvn_trunk/libavcodec/Makefile	2008-04-08 21:17:02.000000000 +1000
> +++ ffmpegsvn_trunk_dirac/libavcodec/Makefile	2008-04-09 16:25:50.000000000 +1000
> @@ -326,11 +330,11 @@
>  OBJS-$(CONFIG_LIBXVID)                 += libxvidff.o libxvid_rc.o
>  
> -
>  OBJS-$(CONFIG_AAC_PARSER)              += aac_parser.o aac_ac3_parser.o mpeg4audio.o

cosmetics

Diego