[FFmpeg-devel] [PATCH] check for mod by zero (issue 2502)

[Justin Ruggles]

On 01/08/2011 01:52 PM, Daniel Kang wrote:

> On Sat, Jan 8, 2011 at 9:33 AM, Justin Ruggles <justin.ruggles at gmail.com>wrote:
> 
>>  On 01/07/2011 11:33 PM, Daniel Kang wrote:
>>> diff --git a/libavformat/vocdec.c b/libavformat/vocdec.c
>>> index 909520c..6c37246 100644
>>> --- a/libavformat/vocdec.c
>>> +++ b/libavformat/vocdec.c
>>> @@ -68,7 +68,7 @@ voc_get_packet(AVFormatContext *s, AVPacket *pkt,
>> AVStream *st, int max_size)
>>>      AVCodecContext *dec = st->codec;
>>>      ByteIOContext *pb = s->pb;
>>>      VocType type;
>>> -    int size;
>>> +    int size, tmp;
>>>      int sample_rate = 0;
>>>      int channels = 1;
>>>
>>> @@ -90,7 +90,11 @@ voc_get_packet(AVFormatContext *s, AVPacket *pkt,
>> AVStream *st, int max_size)
>>>              if (sample_rate)
>>>                  dec->sample_rate = sample_rate;
>>>              dec->channels = channels;
>>> -            dec->codec_id = ff_codec_get_id(ff_voc_codec_tags,
>> get_byte(pb));
>>> +            tmp = ff_codec_get_id(ff_voc_codec_tags, get_byte(pb));
>>> +            if (dec->codec_id != CODEC_ID_NONE)
>>> +                dec->codec_id = ff_codec_get_id(ff_voc_codec_tags,
>> get_byte(pb));
>>> +            else
>>> +                av_log(s, AV_LOG_ERROR, "Unknown codec ID, continuing to
>> decode\n");
>>>              dec->bits_per_coded_sample =
>> av_get_bits_per_sample(dec->codec_id);
>>>              voc->remaining_size -= 2;
>>>              max_size -= 2;
>>> @@ -113,7 +117,11 @@ voc_get_packet(AVFormatContext *s, AVPacket *pkt,
>> AVStream *st, int max_size)
>>>              dec->sample_rate = get_le32(pb);
>>>              dec->bits_per_coded_sample = get_byte(pb);
>>>              dec->channels = get_byte(pb);
>>> -            dec->codec_id = ff_codec_get_id(ff_voc_codec_tags,
>> get_le16(pb));
>>> +            tmp = ff_codec_get_id(ff_voc_codec_tags, get_byte(pb));
>>> +            if (dec->codec_id != CODEC_ID_NONE)
>>> +                dec->codec_id = ff_codec_get_id(ff_voc_codec_tags,
>> get_byte(pb));
>>> +            else
>>> +                av_log(s, AV_LOG_ERROR, "Unknown codec ID, continuing to
>> decode\n");
>>>              url_fskip(pb, 4);
>>>              voc->remaining_size -= 12;
>>>              max_size -= 12;
>>
>>
>> This is not correct.  It is reading the codec code twice. It is not
>> checking to make sure the final codec_id != CODEC_ID_NONE.  And it will
>> never set a valid codec_id. (note that in this case codec_id is not set
>> at read_header() as is normally the case for demuxers. it is set the
>> first time an audio packet is read)
>>
>> My suggestion:
>> switch(){
>> ...
>> case VOC_TYPE_VOICE_DATA:
>>    ...
>>    tmp_codec = ff_codec_get_id(ff_voc_codec_tags, get_byte(pb));
>>    if (dec->codec_id == CODEC_ID_NONE)
>>        dec->codec_id = tmp_codec;
>>    else if (tmp_codec != dec->codec_id)
>>        print AV_LOG_WARNING message about ignoring changed codec code
>>    ...
>> case VOC_TYPE_NEW_VOICE_DATA:
>>    same as above
>>    ...
>> }
>> if (dec->codec_id == CODEC_ID_NONE) {
>>    print error message
>>    return AVERROR(EINVAL);
>>  }
>>
>> -Justin
>>
> 
> I have updated the patch with your comments. I am not sure if
> sample_size can be negative or not, so I have just checked if it is not
> zero.


> +            tmp_codec = ff_codec_get_id(ff_voc_codec_tags, get_byte(pb));
> +            if (dec->codec_id == CODEC_ID_NONE)
> +                dec->codec_id = tmp_codec;
> +            else if (dec->codec_id != tmp_codec)
> +                av_log(s, AV_LOG_WARNING, "Ignoring change in codec, continuing to decode\n");


Technically, the demuxer does not decode, it demuxes.  I would just say
"Ignoring mid-stream change in audio codec".

-Justin