[Ffmpeg-devel] [PATCH] support for VP6 and FLASHSV in the flv muxer

[Reimar Döffinger]

Hello,
On Fri, Dec 15, 2006 at 10:53:30PM +0100, Benjamin Larsson wrote:
> +        switch (enc->codec_id) {
> +            case CODEC_ID_FLV1   :  flags = FLV_CODECID_H263; break;
> +            case CODEC_ID_FLASHSV:  flags = FLV_CODECID_SCREEN; break;
> +            case CODEC_ID_VP6F   :  flags = FLV_CODECID_VP6; break;
> +            case CODEC_ID_VP6    :  flags = FLV_CODECID_VP6; break;

Why duplicate code? just make the CODEC_ID_VP6F fallthrough to
CODEC_ID_VP6.

> +
> +            default:

I personally find it ugly to align the ':', and you don't do it for
"default"...

> -    put_be24(pb,size+1); // include flags
> +    if (enc->codec_id == CODEC_ID_VP6)
> +        put_be24(pb,size+2);
> +    else
> +        put_be24(pb,size+1); // include flags

Both sizes include the flags, so the comment shouldn't be at only one
point.
Not sure if this is more or less ugly:
put_be24(pb,size+1+(enc->codec_id == CODEC_ID_VP6));


>      put_byte(pb,flags);
> +    if (enc->codec_id == CODEC_ID_VP6)
> +        put_byte(pb,0);

Will there always be enough space in the putbyte buffer even with this
additional byte?

Greetings,
Reimar D?ffinger