[MPlayer-dev-eng] [RFC] reindent limpdemux/ aviheader.c and demux_avi.c

[Reimar Döffinger]

Hello,

I pointed out some things that don't have much to do with the reindent,
if nobody else does it I might fix the some day without further warning
so shout if you object.

On Fri, Jul 13, 2007 at 05:09:16PM +0200, Attila Kinali wrote:
[...]
> +extern void print_index(AVIINDEXENTRY * idx, int idx_size,
> +                        int verbose_level);
> +extern void print_avistdindex_chunk(avistdindex_chunk * h,
> +                                    int verbose_level);
> +extern void print_avisuperindex_chunk(avisuperindex_chunk * h,
> +                                      int verbose_level);

IMO should stay on one line. Remove the "extern" instead, it it useless.

>  static int odml_get_vstream_id(int id, unsigned char res[])
>  {
> -    unsigned char *p = (unsigned char *)&id;
> +    unsigned char *p = (unsigned char *) &id;
> +
>      id = le2me_32(id);

I don't really like the extra space, nor does the extra newline seem too
useful.

> -    if (p[2] == 'd') {
> -	if (res) {
> -	    res[0] = p[0];
> -	    res[1] = p[1];
> -	}
> -	return 1;
> +    if (p[2] == 'd')
> +    {
> +        if (res)
> +        {
> +            res[0] = p[0];
> +            res[1] = p[1];
> +        }
> +        return 1;

probably should commented on your lists of options, but IMO { on a
separate line wastes too much space (I don't care for those starting a function
though).

> +                // Indicates where the subject of the file is archived
> +            case mmioFOURCC('I', 'A', 'R', 'L'):
> +                hdr = "Archival Location";
> +                break;
> +                // Lists the artist of the original subject of the file;
> +                // for example, "Michaelangelo."
> +            case mmioFOURCC('I', 'A', 'R', 'T'):

Face it, indent just sucks. The comments are not indented right.

> +                    if (chunksize <= 24)
> +                    {
> +                        break;
> +                    }

it also can't fix useless {} or inconsistently placed comments...

> +                        s->aIndex[i].qwOffset =
> +                            stream_read_dword_le(demuxer->
> +                                                 stream) & 0xffffffff;

What an ugly way to break a line.

> +                        s->aIndex[i].qwOffset |=
> +                            ((uint64_t)
> +                             stream_read_dword_le(demuxer->
> +                                                  stream) & 0xffffffff) <<
> +                            32;

And indent can't make it us the proper stream_read_qword_le instead
either ;-)


> +                        sh_video->bih =
> +                            calloc((chunksize <
> +                                    sizeof(BITMAPINFOHEADER)) ?
> +                                   sizeof(BITMAPINFOHEADER) : chunksize,
> +                                   1);

FFMAX

> +                        stream_read(demuxer->stream,
> +                                    (char *) sh_video->bih, chunksize);

Those stream_read casts are crap. stream_read IMO should just take a
void* argument instead.

> +                        if (sh_video->bih->biSize > chunksize
> +                            && sh_video->bih->biSize >
> +                            sizeof(BITMAPINFOHEADER))

Worst way to break up a long if ever.
I even like Michaels
> if (   sh_video->bih->biSize > chunksize
>     && sh_video->bih->biSize > sizeof(BITMAPINFOHEADER))

better, though I'd go for

> if (sh_video->bih->biSize > chunksize &&
>     sh_video->bih->biSize > sizeof(BITMAPINFOHEADER))

> +                            sh_video->bih->biSize = chunksize;

And the whole statement seems equivalent to
sh_video->bih->biSize = FFMIN(sh_video->bih->biSize, FFMAX(chunksize, sizeof(BITMAPINFOHEADER)));
On second thought that is even uglier though.

Too lazy to look at the rest, but factoring code out would be much, much
more important and would fix the indentation at the same time, without
creating such a mess as indent always does.
Or to put it plainly: you can't replace a maintainer by indent.

Greetings,
Reimar Döffinger