wav file fmt block size could be 16 for formatid==pcm

[Charles Goyard]

Summary of the bug:
ffmpeg produces wav files with a 18 bytes size fmt section (hardcoded in libavformat/riff.c:ff_put_wav_header()). Some software expect the header size to be 16 bytes if wFormatTag is 1 (pcm), as the standard suggests that.
The header size could be 16 for the casual pcm streams, skipping the two optional fields at the end of the fmt section.
This can enhance support for legacy and poorly-written software.

How to reproduce:

% ffmpeg -i in.wav out.wav
% hexdump out.wav | head -2
0000000 4952 4646 0344 0000 4157 4556 6d66 2074
0000010 0012 0000 0001 0001 1f40 0000 3e80 0000
          ^^__ there !
ffmpeg version 1.0 and git master.
built on archlinux

Thank you

[Carl Eugen Hoyos]

Replying to cgo:

ffmpeg produces wav files with a 18 bytes size fmt section (hardcoded in libavformat/riff.c:ff_put_wav_header()). Some software expect the header size to be 16 bytes if wFormatTag is 1 (pcm), as the standard suggests that.

Could you point us to the standard?

[Carl Eugen Hoyos]

Could you test attached patch?
If it works, please name the legacy software.

[Charles Goyard]

Replying to cehoyos:

Could you test attached patch?
If it works, please name the legacy software.

Hi,
the patch does the trick against ffmpeg-1.0, but not against git head.

The difference is that on git master there is now a lot of extra info such as LIST/INFOISFT, encoder name etc included between the fmt and the data section (with and without your patch). I'm not sure this is standard at all?

The software that breaks is pure data, more precisely the wavinfo object, maybe other objects such as readsf~. Note that I also filed a bug report to the puredata project, because the way it reads the header is improper anyway.

The information I found about the wav format are from wikipedia's external links. They all sound like "if format is pcm, header size is 16". I wasn't able to find a authoritative standard, but that's what audacity, sox and to some extend ff_get_wav_header() do.

Thank you for your help.

[Hendrik]

I don't know where 16 bytes comes from, the two relevant structures are WAVEFORMAT and WAVEFORMATEX.

WAVEFORMAT is 14 bytes, WAVEFORMATEX is 18.

Maybe PCMWAVEFORMAT which is a cross between the two, measuring 16 bytes? Its rather unusual though.
To write compliant files it should write a WAVEFORMATEX block with 18 bytes, imho.

[1] WAVEFORMAT: ​http://msdn.microsoft.com/en-us/library/windows/desktop/dd757712(v=vs.85).aspx
[2] WAVEFORMATEX: ​http://msdn.microsoft.com/en-us/library/windows/desktop/dd757713(v=vs.85).aspx
[3] PCMWAVEFORMAT: ​http://msdn.microsoft.com/en-us/library/windows/desktop/dd743663(v=vs.85).aspx

[Carl Eugen Hoyos]

Replying to cgo:

the patch does the trick against ffmpeg-1.0, but not against git head.

Could you try the following with git head and the new patch I will attach (that writes a WAVEFORMAT structure if appropriate)?
$ ffmpeg -i input -map_metadata -1 -flags +bitexact out.wav

[Hendrik]

Plain WAVEFORMAT is no good. You're misinterpreting the code there. WAVEFORMATEXTENSIBLE is yet another extension to WAVEFORMATEX.

The bits per sample member you're cutting off there is required even for 16 bit content, if its not present 8 bit is assumed.

[Carl Eugen Hoyos]

Replying to heleppkes:

The bits per sample member you're cutting off there is required even for 16 bit content, if its not present 8 bit is assumed.

True, patch deleted.
The original patch should allow to write PCMWAVEFORMAT headers though.

[Charles Goyard]

Replying to cehoyos:

The original patch should allow to write PCMWAVEFORMAT headers though.

Right. I reread the wavinfo source code and it really makes wild guesses on how the file is structured:

externals/ext13/wavinfo.c :
typedef struct _wave
{
    char  w_fileid[4];              /* chunk id 'RIFF'            */
    uint32 w_chunksize;             /* chunk size                 */
    char  w_waveid[4];              /* wave chunk id 'WAVE'       */
    char  w_fmtid[4];               /* format chunk id 'fmt '     */
    uint32 w_fmtchunksize;          /* format chunk size          */
    uint16  w_fmttag;               /* format tag, 1 for PCM      */
    uint16  w_nchannels;            /* number of channels         */
    uint32 w_samplespersec;         /* sample rate in hz          */
    uint32 w_navgbytespersec;       /* average bytes per second   */
    uint16  w_nblockalign;          /* number of bytes per sample */
    uint16  w_nbitspersample;       /* number of bits in a sample */
    char  w_datachunkid[4];         /* data chunk id 'data'       */
    uint32 w_datachunksize;         /* length of data chunk       */
} t_wave;

wavinfo = getbytes(sizeof(t_wave));

So any LIST chunk or extended wav format will fail. I think you can merge your patch (it's useful anyway), and close the bug. The problem really lies with this pure-data object.

Thanks for your help and this great software,
Charles

[Carl Eugen Hoyos]

Thank you for looking into the code, patch dropped.