[FFmpeg-devel] [PATCH] adpcm-ima-wav fix bytespersec field in wav header

[Timofei V. Bondarenko]

Michael Niedermayer wrote:
> On Tue, Nov 13, 2007 at 05:00:25PM +0300, Timofei V. Bondarenko wrote:
>> Hi,
>>
>> The patch does write correct bitrate/bytespersecond field in the WAV header 
>> for adpcm-ima-wav codec.
>>
>> Regards,
>> 	Tim.
>>
>> PS
>> I've already sent this patch one week ago but didn't received any comments.
> 
>> Index: libavformat/riff.c
>> ===================================================================
>> --- libavformat/riff.c	(revision 10939)
>> +++ libavformat/riff.c	(working copy)
>> @@ -280,6 +280,8 @@
>>          enc->codec_id == CODEC_ID_PCM_S32LE ||
>>          enc->codec_id == CODEC_ID_PCM_S16LE) {
>>          bytespersec = enc->sample_rate * blkalign;
>> +    } else if (enc->codec_id == CODEC_ID_ADPCM_IMA_WAV) {
>> +        bytespersec = enc->sample_rate * enc->block_align / enc->frame_size;
>>      } else {
>>          bytespersec = enc->bit_rate / 8;
>>      }
> 
> does the following work as well?
> 
>  if (enc->codec_id == CODEC_ID_PCM_U8 ||
>         enc->codec_id == CODEC_ID_PCM_S24LE ||
>         enc->codec_id == CODEC_ID_PCM_S32LE ||
>         enc->codec_id == CODEC_ID_PCM_S16LE ||
> +       enc->enc->block_align

enc->block_align?

>         ) {
>         bytespersec = enc->sample_rate * blkalign;
>     } else {

No, it doesn't.
the block_align is a seek unit which corresponds to 1 frame for simple 
PCM formats, and enc->frame_size frames (about =1017) for adpcm.

> 
> if so, i think it would be preferable as its more general, your solution
> would need a check per codec

I'd prefer a more general solution too because the next thing i want to 
post is a similar fix to avcodec_string().

See the next version:
the pcm_encode_init() set the both frame_size and block_align,
so whole PCM family should be handled right.

I'm not sure about other codecs the new formula becames default for many 
codecs instead of enc->bit_rate/8.
Is (block_align != 0) check safe enough?

At least for WMA this change affects regression.

Regards,
	Tim.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: riff-bps-gen.patch
Type: text/x-patch
Size: 717 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20071114/dfbceab6/attachment.bin>