| Message ID | 20170418223454.10371-1-bobobo@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper <bobobo-at-google.com@ffmpeg.org> 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
On Wed, Apr 19, 2017 at 12:49:38AM +0200, Hendrik Leppkes wrote: > On Wed, Apr 19, 2017 at 12:34 AM, Lucas Cooper > <bobobo-at-google.com@ffmpeg.org> 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'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" <michael@niedermayer.cc> 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 > > <bobobo-at-google.com@ffmpeg.org> 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 > >
Does this need any more work or explanation? On 19 Apr. 2017 7:32 am, "Lucas Cooper" <bobobo@google.com> 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" <michael@niedermayer.cc> > 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 >> > <bobobo-at-google.com@ffmpeg.org> 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 >> >>
Just pinging in case this got lost somewhere. On 21 April 2017 at 00:01, Lucas Cooper <bobobo@google.com> wrote: > Does this need any more work or explanation? > > On 19 Apr. 2017 7:32 am, "Lucas Cooper" <bobobo@google.com> 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" <michael@niedermayer.cc> >> 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 >>> > <bobobo-at-google.com@ffmpeg.org> 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 >>> >>>
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
[...]
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index e6e2313c53..0e337d5258 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1326,7 +1326,10 @@ 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)); + AVRational rational_framerate = find_fps(s, st); + int rate = 0; + if (rational_framerate.den != 0) + rate = av_q2d(rational_framerate); if (!tag) tag = MKTAG('m', '2', 'v', '1'); //fallback tag @@ -1388,7 +1391,10 @@ 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)); + AVRational rational_framerate = find_fps(s, st); + int rate = 0; + if (rational_framerate.den != 0) + rate = av_q2d(rational_framerate); if (!tag) tag = MKTAG('a', 'v', 'c', 'i'); //fallback tag @@ -1850,7 +1856,10 @@ 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)); + AVRational rational_framerate = find_fps(NULL, st); + int rate = 0; + if (rational_framerate.den != 0) + rate = av_q2d(rational_framerate); av_strlcatf(compressor_name, len, "XDCAM"); if (track->par->format == AV_PIX_FMT_YUV422P) { av_strlcatf(compressor_name, len, " HD422");