| Message ID | 1543310306-2050-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
2018-11-27 10:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: > [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid parameter > [libaom-av1 @ 0x469f340] Additional information: g_threads out of range > [..MAX_NUM_THREADS] > > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > --- > libavcodec/libaomenc.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index cb31c55..ccb0cf9 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > enccfg.g_h = avctx->height; > enccfg.g_timebase.num = avctx->time_base.num; > enccfg.g_timebase.den = avctx->time_base.den; > - enccfg.g_threads = avctx->thread_count ? avctx->thread_count : > av_cpu_count(); > + enccfg.g_threads = > + FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 64); Is there a limitation in libaom's api that requires users to never set g_threads >64 that we missed so far? Or is there a bug in libaom that you would like to fix in FFmpeg? Please explain, Carl Eugen
On Wed, Nov 28, 2018 at 4:47 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-11-27 10:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > > fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: > > [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid parameter > > [libaom-av1 @ 0x469f340] Additional information: g_threads out of range > > [..MAX_NUM_THREADS] > > > > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > > --- > > libavcodec/libaomenc.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > index cb31c55..ccb0cf9 100644 > > --- a/libavcodec/libaomenc.c > > +++ b/libavcodec/libaomenc.c > > @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > > enccfg.g_h = avctx->height; > > enccfg.g_timebase.num = avctx->time_base.num; > > enccfg.g_timebase.den = avctx->time_base.den; > > - enccfg.g_threads = avctx->thread_count ? avctx->thread_count : > > av_cpu_count(); > > + enccfg.g_threads = > > + FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), > 64); > > Is there a limitation in libaom's api that requires users to never set > g_threads >64 that we missed so far? > Or is there a bug in libaom that you would like to fix in FFmpeg? > > Please explain, Carl Eugen > > I belive this is a libaom's limitation and we missed the check in FFmpeg part, libvpx have the same issue and the commit d6b1248fc try to fix the same issue.
2018-11-28 1:54 GMT+01:00, mypopy@gmail.com <mypopy@gmail.com>: > On Wed, Nov 28, 2018 at 4:47 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > >> 2018-11-27 10:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: >> > fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: >> > [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid parameter >> > [libaom-av1 @ 0x469f340] Additional information: g_threads out of >> > range >> > [..MAX_NUM_THREADS] >> > >> > Signed-off-by: Jun Zhao <mypopydev@gmail.com> >> > --- >> > libavcodec/libaomenc.c | 3 ++- >> > 1 files changed, 2 insertions(+), 1 deletions(-) >> > >> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >> > index cb31c55..ccb0cf9 100644 >> > --- a/libavcodec/libaomenc.c >> > +++ b/libavcodec/libaomenc.c >> > @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext *avctx, >> > enccfg.g_h = avctx->height; >> > enccfg.g_timebase.num = avctx->time_base.num; >> > enccfg.g_timebase.den = avctx->time_base.den; >> > - enccfg.g_threads = avctx->thread_count ? avctx->thread_count : >> > av_cpu_count(); >> > + enccfg.g_threads = >> > + FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), >> 64); >> >> Is there a limitation in libaom's api that requires users to never set >> g_threads >64 that we missed so far? >> Or is there a bug in libaom that you would like to fix in FFmpeg? >> >> Please explain, Carl Eugen >> >> I belive this is a libaom's limitation Then why isn't the limitation fixed in libaom? Or is it documented? > and we missed the check in FFmpeg part, In which way did we "miss" it, where is it defined that it is invalid to pass large positive numbers to this option? > libvpx have the same issue and the commit d6b1248fc > try to fix the same issue. Two wrong commits don't make a correct one;-) Carl Eugen
On Wed, Nov 28, 2018 at 9:41 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-11-28 1:54 GMT+01:00, mypopy@gmail.com <mypopy@gmail.com>: > > On Wed, Nov 28, 2018 at 4:47 AM Carl Eugen Hoyos <ceffmpeg@gmail.com> > wrote: > > > >> 2018-11-27 10:18 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > >> > fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: > >> > [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid > parameter > >> > [libaom-av1 @ 0x469f340] Additional information: g_threads out of > >> > range > >> > [..MAX_NUM_THREADS] > >> > > >> > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > >> > --- > >> > libavcodec/libaomenc.c | 3 ++- > >> > 1 files changed, 2 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > >> > index cb31c55..ccb0cf9 100644 > >> > --- a/libavcodec/libaomenc.c > >> > +++ b/libavcodec/libaomenc.c > >> > @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext > *avctx, > >> > enccfg.g_h = avctx->height; > >> > enccfg.g_timebase.num = avctx->time_base.num; > >> > enccfg.g_timebase.den = avctx->time_base.den; > >> > - enccfg.g_threads = avctx->thread_count ? avctx > ->thread_count : > >> > av_cpu_count(); > >> > + enccfg.g_threads = > >> > + FFMIN(avctx->thread_count ? avctx->thread_count : a > v_cpu_count(), > >> 64); > >> > >> Is there a limitation in libaom's api that requires users to never set > >> g_threads >64 that we missed so far? > >> Or is there a bug in libaom that you would like to fix in FFmpeg? > >> > >> Please explain, Carl Eugen > >> > >> I belive this is a libaom's limitation > > Then why isn't the limitation fixed in libaom? > Or is it documented? > > This is the check in libaom: https://aomedia.googlesource.com/aom/+/master/av1/av1_cx_iface.c#254 or you can grep "MAX_NUM_THREADS" in libaom :) > > and we missed the check in FFmpeg part, > > In which way did we "miss" it, where is it defined that it is invalid > to pass large positive numbers to this option? > I think we usually don't setting the avctx->thread_count and av_cpu_count() < 64 in most of the CPU, but in our test environment (Intel(R) Xeon(R) Gold 6152) av_cpu_count == 88, this is the reason we didn't find this issue before this case. > > > libvpx have the same issue and the commit d6b1248fc > > try to fix the same issue. > > Two wrong commits don't make a correct one;-) > > Carl Eugen >
On 11/27/2018 6:18 AM, Jun Zhao wrote: > fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: > [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid parameter > [libaom-av1 @ 0x469f340] Additional information: g_threads out of range [..MAX_NUM_THREADS] > > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > --- > libavcodec/libaomenc.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index cb31c55..ccb0cf9 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext *avctx, > enccfg.g_h = avctx->height; > enccfg.g_timebase.num = avctx->time_base.num; > enccfg.g_timebase.den = avctx->time_base.den; > - enccfg.g_threads = avctx->thread_count ? avctx->thread_count : av_cpu_count(); > + enccfg.g_threads = > + FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 64); > > if (ctx->lag_in_frames >= 0) > enccfg.g_lag_in_frames = ctx->lag_in_frames; > Applied. Better to clip the value than to let initialization using default values fail in the user's face because they have a computer with a lot logical cores. libaom should nonetheless either do this clipping internally, or make this limit clear in the API and/or the documentation.
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index cb31c55..ccb0cf9 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -504,7 +504,8 @@ static av_cold int aom_init(AVCodecContext *avctx, enccfg.g_h = avctx->height; enccfg.g_timebase.num = avctx->time_base.num; enccfg.g_timebase.den = avctx->time_base.den; - enccfg.g_threads = avctx->thread_count ? avctx->thread_count : av_cpu_count(); + enccfg.g_threads = + FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 64); if (ctx->lag_in_frames >= 0) enccfg.g_lag_in_frames = ctx->lag_in_frames;
fixed the error in Intel(R) Xeon(R) Gold 6152 CPU like: [libaom-av1 @ 0x469f340] Failed to initialize encoder: Invalid parameter [libaom-av1 @ 0x469f340] Additional information: g_threads out of range [..MAX_NUM_THREADS] Signed-off-by: Jun Zhao <mypopydev@gmail.com> --- libavcodec/libaomenc.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)