[FFmpeg-devel] [PATCH] fix the SAMPLE_FMT_NONE case in ffmdec.c

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Tue, Mar 16, 2010 at 12:19:13PM -0400, Ronald S. Bultje wrote:
>> Hi Peter,
>> 
>> On Tue, Mar 16, 2010 at 2:34 AM, Peter Ross <pross at xvid.org> wrote:
>> > On Mon, Mar 15, 2010 at 04:15:10PM -0400, Ronald S. Bultje wrote:
>> >> --- libavformat/ffmdec.c ? ? ?(revision 22550)
>> >> +++ libavformat/ffmdec.c ? ? ?(working copy)
>> >> @@ -355,6 +355,8 @@
>> >> ? ? ? ? ? ? ?codec->channels = get_le16(pb);
>> >> ? ? ? ? ? ? ?codec->frame_size = get_le16(pb);
>> >
>> >> ? ? ? ? ? ? ?codec->sample_fmt = get_le16(pb);
>> >> + ? ? ? ? ? ?if (codec->sample_fmt == (1 << 16) - 1)
>> >> + ? ? ? ? ? ? ? ?codec->sample_fmt = -1;
>> >
>> > why not read it as a signed 16-bit integer? e.g.
>> > ? ? ? ?codec->sample_fmt = sign_extend( get_le16(), 16 )
>> 
>> True, I didn't know it existed, thanks for the pointer. New patch
>> attached (works the same as previous).
>> 
>> Ronald
>
>>  ffmdec.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 7cc27b8582019e48a0d33b3ee9efbea70e2eb46e  fix-ffm-no_sample_fmt.patch
>> Index: libavformat/ffmdec.c
>> ===================================================================
>> --- libavformat/ffmdec.c	(revision 22565)
>> +++ libavformat/ffmdec.c	(working copy)
>> @@ -21,6 +21,7 @@
>>  
>>  #include "libavutil/intreadwrite.h"
>>  #include "avformat.h"
>> +#include "libavcodec/get_bits.h"
>>  #include "ffm.h"
>>  #if CONFIG_FFSERVER
>>  #include <unistd.h>
>> @@ -354,7 +355,7 @@
>>              codec->sample_rate = get_be32(pb);
>>              codec->channels = get_le16(pb);
>>              codec->frame_size = get_le16(pb);
>> -            codec->sample_fmt = get_le16(pb);
>> +            codec->sample_fmt = sign_extend(get_le16(pb), 16);
>
> a simple cast to int16_t seems simpler

Strictly speaking, that is undefined, but I don't think we care,
especially considering the default sign_extend() implementation also
has undefined behaviour.

-- 
M?ns Rullg?rd
mans at mansr.com