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

[Michael Niedermayer]

On Mon, Mar 30, 2009 at 08:16:26PM +0800, zhentan feng wrote:
> Hi
> 
> 2009/3/30 Michael Niedermayer <michaelni at gmx.at>
> 
> > On Mon, Mar 30, 2009 at 12:29:04AM +0800, zhentan feng wrote:
> > > Hi
> > >
> > > 2009/3/29 Michael Niedermayer <michaelni at gmx.at>
> > >
> > > > On Sun, Mar 29, 2009 at 11:51:57AM +0800, zhentan feng wrote:
> > [...]
> > > > looks broken when extradata_size is not 0
> > > >
> > > >
> > > thanks and here is the new patch attached below.
> > >
> > > --
> > > Best wishes~
> >
> > > Index: libavformat/riff.c
> > > ===================================================================
> > > --- libavformat/riff.c        (revision 18184)
> > > +++ libavformat/riff.c        (working copy)
> > > @@ -282,12 +282,19 @@
> > >  int put_wav_header(ByteIOContext *pb, AVCodecContext *enc)
> > >  {
> > >      int bps, blkalign, bytespersec;
> > > +    int waveformatextensible = 0;
> > >      int hdrsize = 18;
> > > +    int flg = 0;
> > >
> > >      if(!enc->codec_tag || enc->codec_tag > 0xffff)
> > >          return -1;
> > >
> > > -    put_le16(pb, enc->codec_tag);
> > > +    waveformatextensible = enc->channels > 2 && enc->channel_layout;
> > > +    if (waveformatextensible)
> > > +        put_le16(pb, 0xfffe);
> > > +    else
> > > +        put_le16(pb, enc->codec_tag);
> > > +
> > >      put_le16(pb, enc->channels);
> > >      put_le32(pb, enc->sample_rate);
> > >      if (enc->codec_id == CODEC_ID_MP2 || enc->codec_id == CODEC_ID_MP3
> > || enc->codec_id == CODEC_ID_GSM_MS) {
> > > @@ -351,18 +358,35 @@
> > >          put_le16(pb, 2); /* wav_extra_size */
> > >          hdrsize += 2;
> > >          put_le16(pb, enc->frame_size); /* wSamplesPerBlock */
> > > -    } else if(enc->extradata_size){
> > > -        put_le16(pb, enc->extradata_size);
> > > -        put_buffer(pb, enc->extradata, enc->extradata_size);
> > > -        hdrsize += enc->extradata_size;
> > > -        if(hdrsize&1){
> > > -            hdrsize++;
> > > -            put_byte(pb, 0);
> > > -        }
> > > -    } else {
> > > -        hdrsize -= 2;
> > > +    } else
> > > +        flg = 1;
> > > +
> > > +    if (waveformatextensible) {                     /* write
> > WAVEFORMATEXTENSIBLE extensions */
> > > +        put_le16(pb, enc->extradata_size+22);       /* 22 is the size of
> > WAVEFORMATEXTENSIBLE-WAVEFORMATEX */
> > > +        put_le16(pb, enc->bits_per_coded_sample);   /*
> > ValidBitsPerSample || SamplesPerBlock || Reserved */
> > > +        put_le32(pb, enc->channel_layout);          /* dwChannelMask */
> > > +        put_le32(pb, enc->codec_tag);               /* GUID + next 3 */
> > > +        put_le32(pb, 0x00100000);
> > > +        put_le32(pb, 0xAA000080);
> > > +        put_le32(pb, 0x719B3800);
> > > +        hdrsize += 22;
> > >      }
> >
> > i doubt this will work when flg != 1
> >
> > [...]
> 
> 
> here is the new patch for version 0.5.
> according to get_wav_header() fucntion, write the WAVEFORMATEXTENSIBLE
> struct first and update the extrasize if necessary.
> 
[...]
> @@ -343,16 +365,14 @@
>          put_le16(pb, 16); /* fwHeadFlags */
>          put_le32(pb, 0);  /* dwPTSLow */
>          put_le32(pb, 0);  /* dwPTSHigh */
> -    } else if (enc->codec_id == CODEC_ID_GSM_MS) {
> -        put_le16(pb, 2); /* wav_extra_size */
> +    } else if (enc->codec_id == CODEC_ID_GSM_MS || enc->codec_id == CODEC_ID_ADPCM_IMA_WAV) {
> +        if (!waveformatextensible)
> +            put_le16(pb, 2); /* wav_extra_size */
>          hdrsize += 2;
>          put_le16(pb, enc->frame_size); /* wSamplesPerBlock */
> -    } else if (enc->codec_id == CODEC_ID_ADPCM_IMA_WAV) {
> -        put_le16(pb, 2); /* wav_extra_size */
> -        hdrsize += 2;
> -        put_le16(pb, enc->frame_size); /* wSamplesPerBlock */
> -    } else if(enc->extradata_size){
> -        put_le16(pb, enc->extradata_size);
> +    } else if (enc->extradata_size){
> +        if (!waveformatextensible)
> +            put_le16(pb, enc->extradata_size);
>          put_buffer(pb, enc->extradata, enc->extradata_size);
>          hdrsize += enc->extradata_size;
>          if(hdrsize&1){

factorizing code should be in a seperate patch


> @@ -363,6 +383,10 @@
>          hdrsize -= 2;
>      }
>  
> +    if (hdrsize > 40 && waveformatextensible) { /*  40 means 22 WAVEFORMATEXTENSBLE size + 18 */
> +        put_le16(update_pb, hdrsize - 18);
> +    }
> +
>      return hdrsize;
>  }
>  

this is not correct


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

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090330/9d63dce6/attachment.pgp>