Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper wrote: > find_fps attempts to infer framerate from AVCodec's timebase. When this > results in a frame rate that isn't explicitly marked as supported in > av_timecode_check_frame_rate, find_fps returns the AVStream's > avg_frame_rate, which, per avformat.h, _may_ be set (or not). > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and > find_compressor attempt to call av_q2d on the return value of find_fps, > which in the above case, may result in division by zero. Floating point division by zero is not an error (as av_q2d performs), and in these cases the wrong return value is likely harmless. What is the actual error you are trying to fix? - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
On Wed, Apr 19, 2017 at 12:49:38AM +0200, Hendrik Leppkes wrote: > On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper > wrote: > > find_fps attempts to infer framerate from AVCodec's timebase. When this > > results in a frame rate that isn't explicitly marked as supported in > > av_timecode_check_frame_rate, find_fps returns the AVStream's > > avg_frame_rate, which, per avformat.h, _may_ be set (or not). > > > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and > > find_compressor attempt to call av_q2d on the return value of find_fps, > > which in the above case, may result in division by zero. > > Floating point division by zero is not an error (as av_q2d performs), > and in these cases the wrong return value is likely harmless. > > What is the actual error you are trying to fix? from looking at the code, i guess its the cast to int of a non finite double [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
Michael's right. The problem is that NaN is casted to an int, resulting in rate having undefined value. Not sure how I neglected to add that part. On 19 Apr. 2017 2:32 am, "Michael Niedermayer" wrote: > On Wed, Apr 19, 2017 at 12:49:38AM +0200, Hendrik Leppkes wrote: > > On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper > > wrote: > > > find_fps attempts to infer framerate from AVCodec's timebase. When this > > > results in a frame rate that isn't explicitly marked as supported in > > > av_timecode_check_frame_rate, find_fps returns the AVStream's > > > avg_frame_rate, which, per avformat.h, _may_ be set (or not). > > > > > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and > > > find_compressor attempt to call av_q2d on the return value of find_fps, > > > which in the above case, may result in division by zero. > > > > Floating point division by zero is not an error (as av_q2d performs), > > and in these cases the wrong return value is likely harmless. > > > > What is the actual error you are trying to fix? > > from looking at the code, i guess its the cast to int of a non finite > double > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > The real ebay dictionary, page 2 > "100% positive feedback" - "All either got their money back or didnt > complain" > "Best seller ever, very honest" - "Seller refunded buyer after failed scam" > > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
Does this need any more work or explanation? On 19 Apr. 2017 7:32 am, "Lucas Cooper" wrote: > Michael's right. The problem is that NaN is casted to an int, resulting in > rate having undefined value. Not sure how I neglected to add that part. > > On 19 Apr. 2017 2:32 am, "Michael Niedermayer" > wrote: > >> On Wed, Apr 19, 2017 at 12:49:38AM +0200, Hendrik Leppkes wrote: >> > On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper >> > wrote: >> > > find_fps attempts to infer framerate from AVCodec's timebase. When >> this >> > > results in a frame rate that isn't explicitly marked as supported in >> > > av_timecode_check_frame_rate, find_fps returns the AVStream's >> > > avg_frame_rate, which, per avformat.h, _may_ be set (or not). >> > > >> > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and >> > > find_compressor attempt to call av_q2d on the return value of >> find_fps, >> > > which in the above case, may result in division by zero. >> > >> > Floating point division by zero is not an error (as av_q2d performs), >> > and in these cases the wrong return value is likely harmless. >> > >> > What is the actual error you are trying to fix? >> >> from looking at the code, i guess its the cast to int of a non finite >> double >> >> [...] >> >> -- >> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >> >> The real ebay dictionary, page 2 >> "100% positive feedback" - "All either got their money back or didnt >> complain" >> "Best seller ever, very honest" - "Seller refunded buyer after failed >> scam" >> >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
Just pinging in case this got lost somewhere. On 21 April 2017 at 00:01, Lucas Cooper wrote: > Does this need any more work or explanation? > > On 19 Apr. 2017 7:32 am, "Lucas Cooper" wrote: > >> Michael's right. The problem is that NaN is casted to an int, resulting >> in rate having undefined value. Not sure how I neglected to add that part. >> >> On 19 Apr. 2017 2:32 am, "Michael Niedermayer" >> wrote: >> >>> On Wed, Apr 19, 2017 at 12:49:38AM +0200, Hendrik Leppkes wrote: >>> > On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper >>> > wrote: >>> > > find_fps attempts to infer framerate from AVCodec's timebase. When >>> this >>> > > results in a frame rate that isn't explicitly marked as supported in >>> > > av_timecode_check_frame_rate, find_fps returns the AVStream's >>> > > avg_frame_rate, which, per avformat.h, _may_ be set (or not). >>> > > >>> > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and >>> > > find_compressor attempt to call av_q2d on the return value of >>> find_fps, >>> > > which in the above case, may result in division by zero. >>> > >>> > Floating point division by zero is not an error (as av_q2d performs), >>> > and in these cases the wrong return value is likely harmless. >>> > >>> > What is the actual error you are trying to fix? >>> >>> from looking at the code, i guess its the cast to int of a non finite >>> double >>> >>> [...] >>> >>> -- >>> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB >>> >>> The real ebay dictionary, page 2 >>> "100% positive feedback" - "All either got their money back or didnt >>> complain" >>> "Best seller ever, very honest" - "Seller refunded buyer after failed >>> scam" >>> >>> ___ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>> >>> ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
On Fri, Apr 21, 2017 at 12:01:01AM -0700, Lucas Cooper wrote: > Does this need any more work or explanation? The code looks duplicated twice, it should be put in a seperate function, also the indention depth of one of the added lines mismatches the existing code [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB What does censorship reveal? It reveals fear. -- Julian Assange signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
I realized that I missed the indentation. I've fixed it in the latest patch. On 27 April 2017 at 09:52, Lucas Cooper wrote: > find_fps attempts to infer framerate from AVCodec's timebase. When this > results in a frame rate that isn't explicitly marked as supported in > av_timecode_check_frame_rate, find_fps returns the AVStream's > avg_frame_rate, which, per avformat.h, _may_ be set (or not). > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and > find_compressor attempt to call av_q2d on the return value of find_fps, > which in the above case, may result in division by zero and therefore, > an undefined frame rate when NaN is converted to int. > --- > libavformat/movenc.c | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index e6e2313c53..a5c6ab88f1 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -1321,12 +1321,21 @@ static AVRational find_fps(AVFormatContext *s, > AVStream *st) > return rate; > } > > +static int defined_frame_rate(AVFormatContext *s, AVStream *st) > +{ > +AVRational rational_framerate = find_fps(s, st); > +int rate = 0; > +if (rational_framerate.den != 0) > + rate = av_q2d(rational_framerate); > +return rate; > +} > + > static int mov_get_mpeg2_xdcam_codec_tag(AVFormatContext *s, MOVTrack > *track) > { > int tag = track->par->codec_tag; > int interlaced = track->par->field_order > AV_FIELD_PROGRESSIVE; > AVStream *st = track->st; > -int rate = av_q2d(find_fps(s, st)); > +int rate = defined_frame_rate(s, st); > > if (!tag) > tag = MKTAG('m', '2', 'v', '1'); //fallback tag > @@ -1388,7 +1397,7 @@ static int mov_get_h264_codec_tag(AVFormatContext > *s, MOVTrack *track) > int tag = track->par->codec_tag; > int interlaced = track->par->field_order > AV_FIELD_PROGRESSIVE; > AVStream *st = track->st; > -int rate = av_q2d(find_fps(s, st)); > +int rate = defined_frame_rate(s, st); > > if (!tag) > tag = MKTAG('a', 'v', 'c', 'i'); //fallback tag > @@ -1850,7 +1859,7 @@ static void find_compressor(char * compressor_name, > int len, MOVTrack *track) > } else if (track->par->codec_id == AV_CODEC_ID_MPEG2VIDEO && > xdcam_res) { > int interlaced = track->par->field_order > AV_FIELD_PROGRESSIVE; > AVStream *st = track->st; > -int rate = av_q2d(find_fps(NULL, st)); > +int rate = defined_frame_rate(NULL, st); > av_strlcatf(compressor_name, len, "XDCAM"); > if (track->par->format == AV_PIX_FMT_YUV422P) { > av_strlcatf(compressor_name, len, " HD422"); > -- > 2.13.0.rc0.306.g87b477812d-goog > > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.
On Thu, Apr 27, 2017 at 03:08:29PM -0700, Lucas Cooper wrote: > find_fps attempts to infer framerate from AVCodec's timebase. When this > results in a frame rate that isn't explicitly marked as supported in > av_timecode_check_frame_rate, find_fps returns the AVStream's > avg_frame_rate, which, per avformat.h, _may_ be set (or not). > > mov_get_mpeg2_xdcam_codec_tag, mov_get_h264_codec_tag and > find_compressor attempt to call av_q2d on the return value of find_fps, > which in the above case, may result in division by zero and therefore, > an undefined frame rate when NaN is converted to int. > --- > libavformat/movenc.c | 15 --- > 1 file changed, 12 insertions(+), 3 deletions(-) applied thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel