[FFmpeg-devel] FFmpeg source code is no longer C99 because of GNUism called case ranges

[Michael Niedermayer]

Hi

On Wed, Jul 04, 2007 at 08:38:42PM -0700, Roman Shaposhnik wrote:
> On Mon, 2007-06-25 at 09:09 +0100, M?ns Rullg?rd wrote:
> > > My question, of course, is -- would it be acceptable to make code C99
> > > compliant again at an expense of making it slightly less regular?
> > 
> > The main code should be C99 compliant, extensions only allowed under
> > appropriate #ifdefs and always with a proper fallback (think asm()).
> > 
> > Patches, clean of course, to fix issues like the ones you mention are
> > always welcome.
> 
>   Ok. Looks like there's no much disagreement on this one. Thus I'm
> attaching my first cut at making the code C99 compliant again.
> 
>   Michael, please take a look. Does this seem acceptable or would you
> want something fancier?

[...]
> -    switch(avctx->sub_id){
> -    case 0x10000000:
> -        s->rv10_version= 0;
> -        s->low_delay=1;
> -        break;
> -    case 0x10002000:
> -        s->rv10_version= 3;
> -        s->low_delay=1;
> -        s->obmc=1;
> -        break;
> -    case 0x10003000:
> -        s->rv10_version= 3;
> -        s->low_delay=1;
> -        break;
> -    case 0x10003001:
> -        s->rv10_version= 3;
> -        s->low_delay=1;
> -        break;
> -    case 0x20001000: /* real rv20 decoder fail on this id */
> -    /*case 0x20100001:
> -    case 0x20101001:
> -    case 0x20103001:*/
> -    case 0x20100000 ... 0x2019ffff:
> -        s->low_delay=1;
> -        break;
> -    /*case 0x20200002:
> -    case 0x20201002:
> -    case 0x20203002:*/
> -    case 0x20200002 ... 0x202fffff:
> -    case 0x30202002:
> -    case 0x30203002:
> +    if (avctx->sub_id < 0x20000000) {
> +        switch(avctx->sub_id){
> +        case 0x10000000:
> +            s->rv10_version= 0;
> +            s->low_delay=1;
> +            break;
> +        case 0x10002000:
> +            s->rv10_version= 3;
> +            s->low_delay=1;
> +            s->obmc=1;
> +            break;
> +        case 0x10003000:
> +        case 0x10003001:
> +            s->rv10_version= 3;
> +            s->low_delay=1;
> +            break;
> +        default:
> +            av_log(s->avctx, AV_LOG_ERROR, "unknown header %X\n", avctx->sub_id);
> +        }
> +    }

i think it would be more readable if you didnt mix switch with if()


> +    else if (avctx->sub_id == 0x20001000 ||
> +             (avctx->sub_id >= 0x20100000 && avctx->sub_id < 0x201a0000)) {
> +        s->low_delay=1;

vertical align


[...]
> Index: ffmpeg/libavformat/mp3.c
> ===================================================================
> --- ffmpeg/libavformat/mp3.c	(revision 9470)
> +++ ffmpeg/libavformat/mp3.c	(working copy)
> @@ -234,7 +234,8 @@
>          taghdrlen = 6;
>          break;
>  
> -    case 3 ... 4:
> +    case 3:
> +    case 4:
>          isv34 = 1;
>          taghdrlen = 10;
>          break;

ok


[...]
> +        b = probe_packet->buf[i];

b= temp_buffer


[...]
> +        else
> +            res += !((b > 0xAF && b < 0xB7) || (b > 0xB9 && b < 0xC4));

i think
else if(...)
    res++;

would be more readable and not longer


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070705/b6ccf565/attachment.pgp>