Re: [FFmpeg-devel] [PATCH] libkvazaar: Set frame rate as a rational number

2016-01-19 Thread Arttu Ylä-Outinen 

On 2016-01-18 15:46, Nicolas George wrote:


EOVERFLOW does not exist on some windows versions. IIRC, we usually use
EINVAL in this kind of case.



Thanks. I'll change that to EINVAL and apply the patch tomorrow if there 
are no other issues.


- Arttu
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libkvazaar: Set frame rate as a rational number

2016-01-18 Thread Nicolas George 
Le nonidi 29 nivôse, an CCXXIV, Arttu Ylä-Outinen a écrit :
> On 2016-01-16 03:31, Michael Niedermayer wrote:
> 
> >its probably rather unlikely but the multiplication could overflow
> 
> Thanks for taking a look. I attached an updated patch which checks for
> overflow before multiplying.
> 
> - Arttu
> 

> >From 0a8a1a1fffd008d43ec601b7e0a5ed22c2c1f784 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Arttu=20Yl=C3=A4-Outinen?= 
> Date: Fri, 15 Jan 2016 13:47:10 +0200
> Subject: [PATCH v2] libkvazaar: Set frame rate as a rational number
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Updates libkvazaar to pass the exact frame rate to Kvazaar by setting
> the numerator and denominator separately instead of a single floating
> point number. The exact frame rate is needed for writing timing info to
> the bitstream.
> 
> Requires Kvazaar version 0.8.1.
> 
> Signed-off-by: Arttu Yl??-Outinen 
> ---
>  configure   | 2 +-
>  libavcodec/libkvazaar.c | 9 +++--
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index cdf07ae..5ee26cf 100755
> --- a/configure
> +++ b/configure
> @@ -5454,7 +5454,7 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" 
> "gsm/gsm.h"; do
> check_lib "${gsm_hdr}" gsm_create -lgsm 
> && break;
> done || die "ERROR: libgsm not found"; }
>  enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
> -lilbc
> -enabled libkvazaar&& require_pkg_config "kvazaar >= 0.7.1" kvazaar.h 
> kvz_api_get
> +enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h 
> kvz_api_get
>  enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" 
> MFXInit
>  enabled libmodplug&& require_pkg_config libmodplug 
> libmodplug/modplug.h ModPlug_Load
>  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
> lame_set_VBR_quality -lmp3lame
> diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
> index e58405d..8cbc4b0 100644
> --- a/libavcodec/libkvazaar.c
> +++ b/libavcodec/libkvazaar.c
> @@ -75,8 +75,13 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
>  cfg->width  = avctx->width;
>  cfg->height = avctx->height;
>  
> -cfg->framerate =
> -  avctx->time_base.den / (double)(avctx->time_base.num * 
> avctx->ticks_per_frame);
> +if (avctx->ticks_per_frame > INT_MAX / avctx->time_base.num) {
> +av_log(avctx, AV_LOG_ERROR,
> +   "Could not set framerate for kvazaar: integer overflow\n");

> +return AVERROR(EOVERFLOW);

EOVERFLOW does not exist on some windows versions. IIRC, we usually use
EINVAL in this kind of case.

> +}
> +cfg->framerate_num   = avctx->time_base.den;
> +cfg->framerate_denom = avctx->time_base.num * avctx->ticks_per_frame;
>  cfg->target_bitrate = avctx->bit_rate;
>  cfg->vui.sar_width  = avctx->sample_aspect_ratio.num;
>  cfg->vui.sar_height = avctx->sample_aspect_ratio.den;

Regards,

-- 
  Nicolas George
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libkvazaar: Set frame rate as a rational number

2016-01-18 Thread Arttu Ylä-Outinen 

On 2016-01-16 03:31, Michael Niedermayer wrote:


its probably rather unlikely but the multiplication could overflow


Thanks for taking a look. I attached an updated patch which checks for 
overflow before multiplying.


- Arttu

>From 0a8a1a1fffd008d43ec601b7e0a5ed22c2c1f784 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Arttu=20Yl=C3=A4-Outinen?= 
Date: Fri, 15 Jan 2016 13:47:10 +0200
Subject: [PATCH v2] libkvazaar: Set frame rate as a rational number
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Updates libkvazaar to pass the exact frame rate to Kvazaar by setting
the numerator and denominator separately instead of a single floating
point number. The exact frame rate is needed for writing timing info to
the bitstream.

Requires Kvazaar version 0.8.1.

Signed-off-by: Arttu Ylä-Outinen 
---
 configure   | 2 +-
 libavcodec/libkvazaar.c | 9 +++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index cdf07ae..5ee26cf 100755
--- a/configure
+++ b/configure
@@ -5454,7 +5454,7 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" "gsm/gsm.h"; do
check_lib "${gsm_hdr}" gsm_create -lgsm && break;
done || die "ERROR: libgsm not found"; }
 enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode -lilbc
-enabled libkvazaar&& require_pkg_config "kvazaar >= 0.7.1" kvazaar.h kvz_api_get
+enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h kvz_api_get
 enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit
 enabled libmodplug&& require_pkg_config libmodplug libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame
diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index e58405d..8cbc4b0 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -75,8 +75,13 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
 cfg->width  = avctx->width;
 cfg->height = avctx->height;
 
-cfg->framerate =
-  avctx->time_base.den / (double)(avctx->time_base.num * avctx->ticks_per_frame);
+if (avctx->ticks_per_frame > INT_MAX / avctx->time_base.num) {
+av_log(avctx, AV_LOG_ERROR,
+   "Could not set framerate for kvazaar: integer overflow\n");
+return AVERROR(EOVERFLOW);
+}
+cfg->framerate_num   = avctx->time_base.den;
+cfg->framerate_denom = avctx->time_base.num * avctx->ticks_per_frame;
 cfg->target_bitrate = avctx->bit_rate;
 cfg->vui.sar_width  = avctx->sample_aspect_ratio.num;
 cfg->vui.sar_height = avctx->sample_aspect_ratio.den;
-- 
2.7.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH] libkvazaar: Set frame rate as a rational number

2016-01-15 Thread Arttu Ylä-Outinen 
Updates libkvazaar to pass the exact frame rate to Kvazaar by setting
the numerator and denominator separately instead of a single floating
point number. The exact frame rate is needed for writing timing info to
the bitstream.

Requires Kvazaar version 0.8.1.

Signed-off-by: Arttu Ylä-Outinen 
---
 configure   | 2 +-
 libavcodec/libkvazaar.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 7cef6f5..1b004db 100755
--- a/configure
+++ b/configure
@@ -5452,7 +5452,7 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" 
"gsm/gsm.h"; do
check_lib "${gsm_hdr}" gsm_create -lgsm && 
break;
done || die "ERROR: libgsm not found"; }
 enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
-lilbc
-enabled libkvazaar&& require_pkg_config "kvazaar >= 0.7.1" kvazaar.h 
kvz_api_get
+enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h 
kvz_api_get
 enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" MFXInit
 enabled libmodplug&& require_pkg_config libmodplug 
libmodplug/modplug.h ModPlug_Load
 enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
lame_set_VBR_quality -lmp3lame
diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
index e58405d..87b802f 100644
--- a/libavcodec/libkvazaar.c
+++ b/libavcodec/libkvazaar.c
@@ -75,8 +75,8 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
 cfg->width  = avctx->width;
 cfg->height = avctx->height;
 
-cfg->framerate =
-  avctx->time_base.den / (double)(avctx->time_base.num * 
avctx->ticks_per_frame);
+cfg->framerate_num   = avctx->time_base.den;
+cfg->framerate_denom = avctx->time_base.num * avctx->ticks_per_frame;
 cfg->target_bitrate = avctx->bit_rate;
 cfg->vui.sar_width  = avctx->sample_aspect_ratio.num;
 cfg->vui.sar_height = avctx->sample_aspect_ratio.den;
-- 
2.7.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libkvazaar: Set frame rate as a rational number

2016-01-15 Thread Michael Niedermayer 
On Fri, Jan 15, 2016 at 03:42:29PM +0200, Arttu Ylä-Outinen wrote:
> Updates libkvazaar to pass the exact frame rate to Kvazaar by setting
> the numerator and denominator separately instead of a single floating
> point number. The exact frame rate is needed for writing timing info to
> the bitstream.
> 
> Requires Kvazaar version 0.8.1.
> 
> Signed-off-by: Arttu Ylä-Outinen 
> ---
>  configure   | 2 +-
>  libavcodec/libkvazaar.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 7cef6f5..1b004db 100755
> --- a/configure
> +++ b/configure
> @@ -5452,7 +5452,7 @@ enabled libgsm&& { for gsm_hdr in "gsm.h" 
> "gsm/gsm.h"; do
> check_lib "${gsm_hdr}" gsm_create -lgsm 
> && break;
> done || die "ERROR: libgsm not found"; }
>  enabled libilbc   && require libilbc ilbc.h WebRtcIlbcfix_InitDecode 
> -lilbc
> -enabled libkvazaar&& require_pkg_config "kvazaar >= 0.7.1" kvazaar.h 
> kvz_api_get
> +enabled libkvazaar&& require_pkg_config "kvazaar >= 0.8.1" kvazaar.h 
> kvz_api_get
>  enabled libmfx&& require_pkg_config libmfx "mfx/mfxvideo.h" 
> MFXInit
>  enabled libmodplug&& require_pkg_config libmodplug 
> libmodplug/modplug.h ModPlug_Load
>  enabled libmp3lame&& require "libmp3lame >= 3.98.3" lame/lame.h 
> lame_set_VBR_quality -lmp3lame
> diff --git a/libavcodec/libkvazaar.c b/libavcodec/libkvazaar.c
> index e58405d..87b802f 100644
> --- a/libavcodec/libkvazaar.c
> +++ b/libavcodec/libkvazaar.c
> @@ -75,8 +75,8 @@ static av_cold int libkvazaar_init(AVCodecContext *avctx)
>  cfg->width  = avctx->width;
>  cfg->height = avctx->height;
>  
> -cfg->framerate =
> -  avctx->time_base.den / (double)(avctx->time_base.num * 
> avctx->ticks_per_frame);
> +cfg->framerate_num   = avctx->time_base.den;
> +cfg->framerate_denom = avctx->time_base.num * avctx->ticks_per_frame;

its probably rather unlikely but the multiplication could overflow

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel