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

[Michael Niedermayer]

On Wed, Nov 14, 2007 at 03:30:32PM +0300, Timofei V. Bondarenko wrote:
> 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?

yes


>
>>         ) {
>>         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.

if the patch affects regressions then the regression checksums should be
updated in the patch as well

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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20071115/f2a898c4/attachment.pgp>