Re: [FFmpeg-devel] [PATCH] avformat/movenc: Explicitly address potential division by zero.

2017-04-18 Thread Hendrik Leppkes
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.

2017-04-19 Thread Michael Niedermayer
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.

2017-04-19 Thread Lucas Cooper
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.

2017-04-21 Thread Lucas Cooper
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.

2017-04-26 Thread Lucas Cooper
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.

2017-04-27 Thread Michael Niedermayer
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.

2017-04-27 Thread Lucas Cooper
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.

2017-04-28 Thread Michael Niedermayer
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