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

[Daniel Kang]

On Sat, Jan 8, 2011 at 3:45 PM, Justin Ruggles <justin.ruggles at gmail.com>wrote:

> 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
>

I have updated the message.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pcm_sanity_check.diff
Type: application/octet-stream
Size: 3626 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110108/bf2ad11b/attachment.obj>