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

[Måns Rullgård]

Roman Shaposhnik <rvs at sun.com> writes:

> On Thu, 2007-07-05 at 08:02 +0100, M?ns Rullg?rd wrote:
>> >  static int mpeg4video_probe(AVProbeData *probe_packet)
>> >  {
>> >      uint32_t temp_buffer= -1;
>> > +    uint8_t b;
>> >      int VO=0, VOL=0, VOP = 0, VISO = 0, res=0;
>> >      int i;
>> >  
>> >      for(i=0; i<probe_packet->buf_size; i++){
>> >          temp_buffer = (temp_buffer<<8) + probe_packet->buf[i];
>> > -        if ((temp_buffer & 0xffffff00) == 0x100) {
>> > -            switch(temp_buffer){
>> > -            case VOP_START_CODE:             VOP++; break;
>> > -            case VISUAL_OBJECT_START_CODE:  VISO++; break;
>> > -            case 0x100 ... 0x11F:             VO++; break;
>> > -            case 0x120 ... 0x12F:            VOL++; break;
>> > -            case 0x130 ... 0x1AF:
>> > -            case 0x1B7 ... 0x1B9:
>> > -            case 0x1C4 ... 0x1FF:            res++; break;
>> > -            }
>> > -        }
>> > +        if ((temp_buffer & 0xffffff00) != 0x100)
>> > +            continue;
>> > +
>> > +        b = probe_packet->buf[i];
>> > +        if (b == VOP_START_CODE)
>> > +            VOP++;
>> > +        else if (b == VISUAL_OBJECT_START_CODE)
>> > +            VISO++;
>> > +        else if (b < 0x20)
>> > +            VO++;
>> > +        else if (b < 0x30)
>> > +            VOL++;
>> > +        else
>> > +            res += !((b > 0xAF && b < 0xB7) || (b > 0xB9 && b < 0xC4));
>> >      }
>> >  
>> >      if ( VOP >= VISO && VOP >= VOL && VO >= VOL && VOL > 0 && res==0)
>> 
>> Why the new variable?  The logic looks correct though.
>
>   As a precaution and cosmetics. Precaution against stupid compilers,

Please elaborate.  I thought your compilers were supposed to be good ;-)
Using only the low 8 bits would have a slight advantage on ARM though,
since literal operands can only have 8 consecutive non-zero bits.
This code isn't speed-critical though so I wouldn't worry too much
about such things.

> cosmetics because:
>         if (b == ....)
> looks nicer (IMHO) than:
>         if ((temp_buffer & 0xffffff00) == ...)

You don't need the mask.  It's enough to do

    if (temp_buffer == ...)
    ...
    else if (temp_buffer < 0x120)
    ...

You've already determined that the high 23 bits are all zero.

-- 
M?ns Rullg?rd
mans at mansr.com