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

[Roman Shaposhnik]

On Thu, 2007-07-05 at 08:02 +0100, M?ns Rullg?rd wrote:
> >      if(avctx->debug & FF_DEBUG_PICT_INFO){
> >          av_log(avctx, AV_LOG_DEBUG, "ver:%X ver0:%X\n", avctx->sub_id, avctx->extradata_size >= 4 ? ((uint32_t*)avctx->extradata)[0] : -1);
> 
> This has the appearance of something that could possibly be done with
> some bit masking and arithmetic instead of listing all the
> possibilities.

  I have the very same feeling. That's why I asked somebody familiar
with this codec to take a look. The most complicated part would 
be to still issue a message when we see an unfamiliar header, though.
The question is: do we even want to? I'd say no.

> >  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,
cosmetics because:
        if (b == ....)
looks nicer (IMHO) than:
        if ((temp_buffer & 0xffffff00) == ...)

Thanks,
Roman.