[FFmpeg-devel] [PATCH] check for mod by zero (issue 2502)
Michael Niedermayer
michaelni
Mon Jan 10 21:51:46 CET 2011
On Sat, Jan 08, 2011 at 04:39:35PM -0500, Daniel Kang wrote:
> 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.
> libavcodec/pcm.c | 10 ++++++++++
> libavformat/vocdec.c | 18 +++++++++++++++---
> 2 files changed, 25 insertions(+), 3 deletions(-)
> 65c7d12fe75a01827377e1adb4dfa2a7b7015dd6 pcm_sanity_check.diff
> From f9905f8b8c077d079951344c7d25646589c4c5aa Mon Sep 17 00:00:00 2001
> From: Daniel Kang <daniel.d.kang at gmail.com>
> Date: Thu, 6 Jan 2011 21:03:27 -0500
> Subject: [PATCH] Add check for pcm files.
>
> ---
> libavcodec/pcm.c | 10 ++++++++++
> libavformat/vocdec.c | 18 +++++++++++++++---
> 2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/pcm.c b/libavcodec/pcm.c
> index b6b49dc..e321a4d 100644
> --- a/libavcodec/pcm.c
> +++ b/libavcodec/pcm.c
> @@ -272,6 +272,11 @@ static int pcm_decode_frame(AVCodecContext *avctx,
> samples = data;
> src = buf;
>
> + if (avctx->codec_id == CODEC_ID_NONE) {
> + av_log(avctx, AV_LOG_ERROR, "invalid codec_id\n");
> + return AVERROR(EINVAL);
> + }
> +
> if (avctx->sample_fmt!=avctx->codec->sample_fmts[0]) {
> av_log(avctx, AV_LOG_ERROR, "invalid sample_fmt\n");
> return -1;
> @@ -292,6 +297,11 @@ static int pcm_decode_frame(AVCodecContext *avctx,
> /* we process 40-bit blocks per channel for LXF */
> sample_size = 5;
>
> + if (sample_size == 0) {
> + av_log(avctx, AV_LOG_ERROR, "Invalid sample_size\n");
> + return AVERROR(EINVAL);
> + }
the codec_id test looks unneeded
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you really think that XML is the answer, then you definitly missunderstood
the question -- Attila Kinali
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20110110/a750ae44/attachment.pgp>
More information about the ffmpeg-devel
mailing list