[FFmpeg-devel] Fwd: [PATCH] Psygnosis YOP demuxer

[Diego Biurrun]

On Sat, Dec 26, 2009 at 07:07:45PM +0530, Mohamed Naufal wrote:
> 2009/12/26 Diego Biurrun <diego at biurrun.de>
> [...]
> 
> > > +    int palette_size = 4 + yop->num_pal_colors * 3;
> > > +    int actual_video_data_size =
> > > +        yop->frame_size - yop->sound_chunk_length - palette_size;
> >
> > ugly formatting again
> >
> > I apologize for making you repeat yourself.

This apology sounds very hollow since you are making me repeat myself
once again: I said your patch still contained trailing whitespace and
tabs even though Michael already pointed this out in his first mail.

Fix it.

> --- libavformat/yop.c	(revision 0)
> +++ libavformat/yop.c	(revision 0)
> @@ -0,0 +1,177 @@
> +
> +static int yop_probe(AVProbeData *probe_packet)
> +{
> +    if ((AV_RB16(probe_packet->buf) == AV_RB16("YO")) &&
> +           (probe_packet->buf[6] != 0) && (probe_packet->buf[7] != 0) &&
> +               !(AV_RL16(probe_packet->buf + 8) & 1) && !(AV_RL16(probe_packet->buf + 10) & 1))
> +            
> +        return AVPROBE_SCORE_MAX/2 + 1;
> +	
> +    return 0;

- The indentation of the if statement is off.  Align with the first
  character after the opening parenthesis.
- Some of the parentheses in that expression are superfluous while not
  helping readability IMO (the ones not preceded by !).
- The '!= 0' could possibly be skipped as well, but do as you wish.
- The empty line is confusing me.
- I would place spaces around the /.

> +        else if (ret < palette_size) {

Merge these two lines.

> +        }
> +        else if (ret < actual_video_data_size) {

ditto

Diego

P.S.: Place empty lines between the quoted text and your reply.  If you
      write on the same lines as the quotation marks, your reply appears
      to be quoted text.