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

Diego Biurrun diego
Sat Dec 26 15:53:12 CET 2009


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.



More information about the ffmpeg-devel mailing list