[FFmpeg-devel] [PATCH] RPL demuxer

[Aurelien Jacobs]

On Fri, 2 Nov 2007 16:33:45 +0100
Christian Ohm <chr.ohm at gmx.net> wrote:

> 
> Here's a patch for a RPL demuxer. It plays landing.rpl from the samples,
> and several other files from Warzone 2100 (sound only, since there's no
> video decoder yet).
> 
> diff -r 75b6fd3fe06a libavformat/rpl.c
> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
> +++ b/libavformat/rpl.c	Fri Nov 02 15:13:04 2007 +0100
> @@ -0,0 +1,335 @@
> +/*
> + * ARMovie/RPL demuxer
> + */

Please use a standard license header.

> +#define RPL_SIGNATURE "ARMovie\x0A"
> +#define RPL_SIGNATURE_SIZE 8
> +
> +#define RPL_TAG1 MKTAG('A', 'R', 'M', 'o')
> +#define RPL_TAG2 MKTAG('v', 'i', 'e', '\x0A')
> +
> +static int rpl_probe(AVProbeData *p)
> +{
> +	if (p->buf_size < 2048)
> +		return 0;

Tabs are forbiden in our repository. Please use 4 spaces indentation.

> +	if ((AV_RL32(&p->buf[0]) != RPL_TAG1) || (AV_RL32(&p->buf[4]) != RPL_TAG2))

Here you can probably simplify with something like:
    if (memcmp(p->buf, RPL_SIGNATURE, 8))

> +	for (i = 0; i < 21; i++)
> +	{

Please be consistent with { place. In other part of this file it's
place on the same line as the for, if, or whatever (which is the
prefered syntax in ffmpeg).

> +	rpl->author = b;
> +
> +	i++;
> +
> +	sscanf(buffer + so[i], "%i", &rpl->video_format);
> +
> +	i++;
> +
> +	sscanf(buffer + so[i], "%i", &rpl->video_width);

All those i++ lines and empty lines make the code much longer than needed.
I would prefer seeing something like:

+	sscanf(buffer + so[i++], "%i", &rpl->video_format);
+	sscanf(buffer + so[i++], "%i", &rpl->video_width);

> +AVInputFormat rpl_demuxer = {
> +	"rpl",
> +	"RPL/ARMovie format",
> +	sizeof(RPLContext),
> +	rpl_probe,
> +	rpl_read_header,
> +	rpl_read_packet,
> +	rpl_read_close,
> +};
> +

Useless empty line.

Aurel