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

[Ronald S. Bultje]

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.

Ronald