[Ffmpeg-devel] [PATCH] FLV common header.

[Michael Niedermayer]

Hi

On Sun, Dec 03, 2006 at 12:44:28AM -0800, Allan Hsu wrote:
> 
> On Dec 2, 2006, at 3:13 AM, Michael Niedermayer wrote:
> 
> >Hi
> >
> >On Fri, Dec 01, 2006 at 08:04:54PM -0800, Allan Hsu wrote:
> >>The attached patch adds a common header, flv.h, that defines various
> >>common FLV container values between the FLV muxer and demuxer. It
> >>moves some defines from the muxer to enums in the common header and
> >>it changes the hardcoded values in both flvenc.c and flvdec.c to the
> >>new values in the common header. It should not change the behavior of
> >>any of the code.
> >>
> >>If this patch is acceptable, my intention is to use the common header
> >>values in a forthcoming series of onMetaData tag reading patches.
> >
> >patch ok except a few nitpicks
> 
> [...]
> 
> Here's the same patch with your suggestions taken into account.
> 
[...]
>              st->codec->codec_type = CODEC_TYPE_VIDEO;
> -            switch(flags & 0xF){
> -            case 2: st->codec->codec_id = CODEC_ID_FLV1; break;
> -            case 3: st->codec->codec_id = CODEC_ID_FLASHSV; break;
> -            case 4:
> +            switch(flags & FLV_VIDEO_CODECID_MASK){
> +            case FLV_CODECID_H263: st->codec->codec_id = CODEC_ID_FLV1; break;
> +            case FLV_CODECID_SCREEN: st->codec->codec_id = CODEC_ID_FLASHSV; break;

please align these like:

            case FLV_CODECID_H263  : st->codec->codec_id = CODEC_ID_FLV1   ; break;
            case FLV_CODECID_SCREEN: st->codec->codec_id = CODEC_ID_FLASHSV; break;

same for the other similar code


[...]

> +enum {
> +    FLV_CODECID_PCM_BE = 0,
> +    FLV_CODECID_ADPCM = 1 << FLV_AUDIO_CODECID_OFFSET,
> +    FLV_CODECID_MP3 = 2 << FLV_AUDIO_CODECID_OFFSET,
> +    FLV_CODECID_PCM_LE = 3 << FLV_AUDIO_CODECID_OFFSET,
> +    FLV_CODECID_NELLYMOSER_8HZ_MONO = 5 << FLV_AUDIO_CODECID_OFFSET,
> +    FLV_CODECID_NELLYMOSER = 6 << FLV_AUDIO_CODECID_OFFSET

please align these like:

   FLV_CODECID_ADPCM               = 1 << FLV_AUDIO_CODECID_OFFSET,
   FLV_CODECID_MP3                 = 2 << FLV_AUDIO_CODECID_OFFSET,
   FLV_CODECID_PCM_LE              = 3 << FLV_AUDIO_CODECID_OFFSET,
   FLV_CODECID_NELLYMOSER_8HZ_MONO = 5 << FLV_AUDIO_CODECID_OFFSET,
   FLV_CODECID_NELLYMOSER          = 6 << FLV_AUDIO_CODECID_OFFSET

makes them look much nicer, same for all the others


except these the patch is ok

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

If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali