| Message ID | 20181027205214.75673-1-jzern@google.com |
|---|---|
| State | Accepted |
| Headers | show |
On 10/27/2018 5:52 PM, James Zern wrote: > a thread count of 0 is treated the same as 1, use av_cpu_count() to get > the correct thread count when auto threads is requested. > > this matches the fix in libvpxenc: > 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for encoding > > Signed-off-by: James Zern <jzern@google.com> > --- > libavcodec/libvpxdec.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c > index 04f27d3396..5193ac51c3 100644 > --- a/libavcodec/libvpxdec.c > +++ b/libavcodec/libvpxdec.c > @@ -47,8 +47,7 @@ static av_cold int vpx_init(AVCodecContext *avctx, > { > VPxContext *ctx = avctx->priv_data; > struct vpx_codec_dec_cfg deccfg = { > - /* token partitions+1 would be a decent choice */ > - .threads = FFMIN(avctx->thread_count, 16) > + .threads = avctx->thread_count ? avctx->thread_count : av_cpu_count() If the limit of 16 threads is correct and unless libvpx clips it on its own, then you shouldn't remove the check. LGTM otherwise. > }; > > av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str()); >
On Sat, Oct 27, 2018 at 1:54 PM James Almer <jamrial@gmail.com> wrote: > > On 10/27/2018 5:52 PM, James Zern wrote: > > a thread count of 0 is treated the same as 1, use av_cpu_count() to get > > the correct thread count when auto threads is requested. > > > > this matches the fix in libvpxenc: > > 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for encoding > > > > Signed-off-by: James Zern <jzern@google.com> > > --- > > libavcodec/libvpxdec.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c > > index 04f27d3396..5193ac51c3 100644 > > --- a/libavcodec/libvpxdec.c > > +++ b/libavcodec/libvpxdec.c > > @@ -47,8 +47,7 @@ static av_cold int vpx_init(AVCodecContext *avctx, > > { > > VPxContext *ctx = avctx->priv_data; > > struct vpx_codec_dec_cfg deccfg = { > > - /* token partitions+1 would be a decent choice */ > > - .threads = FFMIN(avctx->thread_count, 16) > > + .threads = avctx->thread_count ? avctx->thread_count : av_cpu_count() > > If the limit of 16 threads is correct and unless libvpx clips it on its > own, then you shouldn't remove the check. > If anything it was a limit put in at one stage of vp8 development, the comment is certainly vp8 specific. vp8/9 both clip in their own ways [1][2], but looking at it there may have been some changes to the loop filter that aren't limiting this number. I'll restore it to be safe. On the libvpxenc side I don't think there was ever a check in the wrapper, but the library does put a hard cap on the requested threads (64) and will fail otherwise. [1] https://chromium.googlesource.com/webm/libvpx/+/v1.7.0/vp8/decoder/threading.c#596 [2] https://chromium.googlesource.com/webm/libvpx/+/v1.7.0/vp9/decoder/vp9_decodeframe.c#1542 > LGTM otherwise. > > > }; > > > > av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str()); > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
On Mon, Oct 29, 2018 at 10:58 PM James Zern <jzern@google.com> wrote: > > On Sat, Oct 27, 2018 at 1:54 PM James Almer <jamrial@gmail.com> wrote: > > > > On 10/27/2018 5:52 PM, James Zern wrote: > > > a thread count of 0 is treated the same as 1, use av_cpu_count() to get > > > the correct thread count when auto threads is requested. > > > > > > this matches the fix in libvpxenc: > > > 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for encoding > > > > > > Signed-off-by: James Zern <jzern@google.com> > > > --- > > > libavcodec/libvpxdec.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > Applied with the limit of 16 restored. Thanks for the review.
diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c index 04f27d3396..5193ac51c3 100644 --- a/libavcodec/libvpxdec.c +++ b/libavcodec/libvpxdec.c @@ -47,8 +47,7 @@ static av_cold int vpx_init(AVCodecContext *avctx, { VPxContext *ctx = avctx->priv_data; struct vpx_codec_dec_cfg deccfg = { - /* token partitions+1 would be a decent choice */ - .threads = FFMIN(avctx->thread_count, 16) + .threads = avctx->thread_count ? avctx->thread_count : av_cpu_count() }; av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str());
a thread count of 0 is treated the same as 1, use av_cpu_count() to get the correct thread count when auto threads is requested. this matches the fix in libvpxenc: 27df34bf1f avcodec/libvpxenc: fix setting amount of threads used for encoding Signed-off-by: James Zern <jzern@google.com> --- libavcodec/libvpxdec.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)