[FFmpeg-devel] Add waveformat extensible support in wav muxer (SoC qualification task)

[zhentan feng]

Hi

2009/4/2 Ronald S. Bultje <rsbultje at gmail.com>

> Hi Zhentan,
>
> On Thu, Apr 2, 2009 at 7:28 AM, zhentan feng <spyfeng at gmail.com> wrote:
> > here is the new patch attached below.
>
> This looks better, IMO, and you'll notice the patch is smaller also.
> One thing you want to consider in general, is to separate indenting
> changes:
>
> -    put_le16(pb, enc->codec_tag);
> +    waveformatextensible = enc->channels > 2 && enc->channel_layout;
> +    if (waveformatextensible) {
> +        put_le16(pb, 0xfffe);
> +        pre_size += 22; /* 22 is the size of
> WAVEFORMATEXTENSIBLE-WAVEFORMATEX */
> +    } else
> +        put_le16(pb, enc->codec_tag);
>
> You'll notice the first and last line remove and re-add the same line,
> so what you want to do is create two patches, one that just leaves it:
>
> +    waveformatextensible = enc->channels > 2 && enc->channel_layout;
> +    if (waveformatextensible) {
> +        put_le16(pb, 0xfffe);
> +        pre_size += 22; /* 22 is the size of
> WAVEFORMATEXTENSIBLE-WAVEFORMATEX */
> +    } else
>      put_le16(pb, enc->codec_tag);
>
> And a second one which reindents it:
>
> -   put_le16(pb, enc->codec_tag);
> +       put_le16(pb, enc->codec_tag);
>
> Normally, I (induced by Michael :-) ) would in fact recommend to
> separate your patch in multiple smaller ones anyway, e.g. one that
> moves the writing of the put_le16() away from the codec-specific area
> upwards, and then a second one which actually adds the
> WAVEFORMATEXTENSIBLE around it. I guess in this case they're sort of
> interconnected so I'll live with it.
>

thanks, I'll pay more attention to the rules


zhentan

-- 
Best wishes~