Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
@ 2023-01-05 10:41 Dmitrii Ovchinnikov
  2023-01-05 17:14 ` Tomas Härdin
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Dmitrii Ovchinnikov @ 2023-01-05 10:41 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Dmitrii Ovchinnikov

From: Dmitrii Ovchinnikov <ovchinnikov.dmitrii@gmail.com>

This change improves the performance and multicore
 scalability of the vp9 codec for streaming single-pass encoded videos. The
 current thread limit for ffmpeg codecs is 16 (MAX_AUTO_THREADS in
 pthread_internal.h) due to a limitation in H.264 codec that prevents more
 than 16 threads being used.

Increasing the thread limit to 64 for vp9 improves
the performance for encoding 4K raw videos for streaming by up to 47%
compared to 16 threads, and from 20-30% for 32 threads, with the same quality
as measured by the VMAF score.

Did not need to add a check for limit in libvpx as it is already present 
in libvpx/vp9/vp9_cx_iface.c:
  RANGE_CHECK_HI(cfg, g_threads, 64);
As demonstrated by following message when -threads is set to anything more than 64
[libvpx-vp9 @ 0x30ed380]   Additional information: g_threads out of range [..64]

---
 libavcodec/libvpxdec.c | 2 +-
 libavcodec/libvpxenc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
index 9cd2c56caf..19407092d0 100644
--- a/libavcodec/libvpxdec.c
+++ b/libavcodec/libvpxdec.c
@@ -88,7 +88,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
                             const struct vpx_codec_iface *iface)
 {
     struct vpx_codec_dec_cfg deccfg = {
-        .threads = FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16)
+        .threads = avctx->thread_count ? avctx->thread_count : av_cpu_count()
     };
 
     av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str());
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 9aa5510c28..0627e13973 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -942,7 +942,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
     enccfg.g_timebase.num = avctx->time_base.num;
     enccfg.g_timebase.den = avctx->time_base.den;
     enccfg.g_threads      =
-        FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16);
+        avctx->thread_count ? avctx->thread_count : av_cpu_count();
     enccfg.g_lag_in_frames= ctx->lag_in_frames;
 
     if (avctx->flags & AV_CODEC_FLAG_PASS1)
-- 
2.38.1.windows.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
  2023-01-05 10:41 [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit Dmitrii Ovchinnikov
@ 2023-01-05 17:14 ` Tomas Härdin
  2023-01-10  3:06 ` James Zern
  2023-01-10 10:47 ` mypopy
  2 siblings, 0 replies; 6+ messages in thread
From: Tomas Härdin @ 2023-01-05 17:14 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

tor 2023-01-05 klockan 11:41 +0100 skrev Dmitrii Ovchinnikov:
> From: Dmitrii Ovchinnikov <ovchinnikov.dmitrii@gmail.com>
> 
> This change improves the performance and multicore
>  scalability of the vp9 codec for streaming single-pass encoded
> videos. The
>  current thread limit for ffmpeg codecs is 16 (MAX_AUTO_THREADS in
>  pthread_internal.h) due to a limitation in H.264 codec that prevents
> more
>  than 16 threads being used.

This limitation should be restricted to H.264 IMO, not applied to all
codecs wholesale. I ran into this issue with jpeg2000dec.

> 
> Increasing the thread limit to 64 for vp9 improves
> the performance for encoding 4K raw videos for streaming by up to 47%
> compared to 16 threads, and from 20-30% for 32 threads, with the same
> quality
> as measured by the VMAF score.
> 
> Did not need to add a check for limit in libvpx as it is already
> present 
> in libvpx/vp9/vp9_cx_iface.c:
>   RANGE_CHECK_HI(cfg, g_threads, 64);

Perhaps we should have a table of known max number of threads per
codec?

/Tomas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
  2023-01-05 10:41 [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit Dmitrii Ovchinnikov
  2023-01-05 17:14 ` Tomas Härdin
@ 2023-01-10  3:06 ` James Zern
  2023-01-10 10:47 ` mypopy
  2 siblings, 0 replies; 6+ messages in thread
From: James Zern @ 2023-01-10  3:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Dmitrii Ovchinnikov

On Thu, Jan 5, 2023 at 2:42 AM Dmitrii Ovchinnikov
<ovchinnikov.dmitrii@gmail.com> wrote:
>
> From: Dmitrii Ovchinnikov <ovchinnikov.dmitrii@gmail.com>
>
> This change improves the performance and multicore
>  scalability of the vp9 codec for streaming single-pass encoded videos. The
>  current thread limit for ffmpeg codecs is 16 (MAX_AUTO_THREADS in
>  pthread_internal.h) due to a limitation in H.264 codec that prevents more
>  than 16 threads being used.
>
> Increasing the thread limit to 64 for vp9 improves
> the performance for encoding 4K raw videos for streaming by up to 47%
> compared to 16 threads, and from 20-30% for 32 threads, with the same quality
> as measured by the VMAF score.
>
> Did not need to add a check for limit in libvpx as it is already present
> in libvpx/vp9/vp9_cx_iface.c:
>   RANGE_CHECK_HI(cfg, g_threads, 64);
> As demonstrated by following message when -threads is set to anything more than 64
> [libvpx-vp9 @ 0x30ed380]   Additional information: g_threads out of range [..64]
>

The difference is that it will cause a failure rather than silently
capping the value which might break some workflows. libaomenc caps at
64, so it may be best to match that implementation for now. I'll make
that change before submitting unless there are other comments.

With this change there will still be the misleading output from
validate_thread_parameters() in pthread.c, though it's not a unique
problem to libvpxenc:
[libvpx-vp9 @ 0x5639aa4c0200] Application has requested 64 threads.
Using a thread count greater than 16 is not recommended.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
  2023-01-05 10:41 [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit Dmitrii Ovchinnikov
  2023-01-05 17:14 ` Tomas Härdin
  2023-01-10  3:06 ` James Zern
@ 2023-01-10 10:47 ` mypopy
  2023-01-11  1:23   ` James Zern
  2 siblings, 1 reply; 6+ messages in thread
From: mypopy @ 2023-01-10 10:47 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Thu, Jan 5, 2023 at 6:42 PM Dmitrii Ovchinnikov
<ovchinnikov.dmitrii@gmail.com> wrote:
>
> From: Dmitrii Ovchinnikov <ovchinnikov.dmitrii@gmail.com>
>
> This change improves the performance and multicore
>  scalability of the vp9 codec for streaming single-pass encoded videos. The
>  current thread limit for ffmpeg codecs is 16 (MAX_AUTO_THREADS in
>  pthread_internal.h) due to a limitation in H.264 codec that prevents more
>  than 16 threads being used.
>
> Increasing the thread limit to 64 for vp9 improves
> the performance for encoding 4K raw videos for streaming by up to 47%
> compared to 16 threads, and from 20-30% for 32 threads, with the same quality
> as measured by the VMAF score.
>
> Did not need to add a check for limit in libvpx as it is already present
> in libvpx/vp9/vp9_cx_iface.c:
>   RANGE_CHECK_HI(cfg, g_threads, 64);
> As demonstrated by following message when -threads is set to anything more than 64
> [libvpx-vp9 @ 0x30ed380]   Additional information: g_threads out of range [..64]
>
> ---
>  libavcodec/libvpxdec.c | 2 +-
>  libavcodec/libvpxenc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/libvpxdec.c b/libavcodec/libvpxdec.c
> index 9cd2c56caf..19407092d0 100644
> --- a/libavcodec/libvpxdec.c
> +++ b/libavcodec/libvpxdec.c
> @@ -88,7 +88,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
>                              const struct vpx_codec_iface *iface)
>  {
>      struct vpx_codec_dec_cfg deccfg = {
> -        .threads = FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16)
> +        .threads = avctx->thread_count ? avctx->thread_count : av_cpu_count()
>      };
>
>      av_log(avctx, AV_LOG_INFO, "%s\n", vpx_codec_version_str());
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 9aa5510c28..0627e13973 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -942,7 +942,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
>      enccfg.g_timebase.num = avctx->time_base.num;
>      enccfg.g_timebase.den = avctx->time_base.den;
>      enccfg.g_threads      =
> -        FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16);
> +        avctx->thread_count ? avctx->thread_count : av_cpu_count();
>      enccfg.g_lag_in_frames= ctx->lag_in_frames;
>
Do you check the change with the old version libvpx?  as I know, older
versions libvpx setting the number of threads higher than 16 will
cause a crash, so I think at least a version check needs to be added

>      if (avctx->flags & AV_CODEC_FLAG_PASS1)
> --
> 2.38.1.windows.1
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
  2023-01-10 10:47 ` mypopy
@ 2023-01-11  1:23   ` James Zern
  2023-01-17 22:06     ` James Zern
  0 siblings, 1 reply; 6+ messages in thread
From: James Zern @ 2023-01-11  1:23 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Jan 10, 2023 at 2:47 AM mypopy@gmail.com <mypopy@gmail.com> wrote:
>
> On Thu, Jan 5, 2023 at 6:42 PM Dmitrii Ovchinnikov
> <ovchinnikov.dmitrii@gmail.com> wrote:
> [...]
> > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > index 9aa5510c28..0627e13973 100644
> > --- a/libavcodec/libvpxenc.c
> > +++ b/libavcodec/libvpxenc.c
> > @@ -942,7 +942,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
> >      enccfg.g_timebase.num = avctx->time_base.num;
> >      enccfg.g_timebase.den = avctx->time_base.den;
> >      enccfg.g_threads      =
> > -        FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16);
> > +        avctx->thread_count ? avctx->thread_count : av_cpu_count();
> >      enccfg.g_lag_in_frames= ctx->lag_in_frames;
> >
> Do you check the change with the old version libvpx?  as I know, older
> versions libvpx setting the number of threads higher than 16 will
> cause a crash, so I think at least a version check needs to be added
>

Do you have a bug or version in mind? There were some performance
regressions [1] over the releases and some issues with changing the
number of tiles, but I don't remember a crash for a high thread count
(though there have been plenty of crashes related to threads in
general [2]). The range check predates 1.4.0, which is the minimum
required by ffmpeg.

[1] https://crbug.com/webm/1574
[2] https://crbug.com/webm/851
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit
  2023-01-11  1:23   ` James Zern
@ 2023-01-17 22:06     ` James Zern
  0 siblings, 0 replies; 6+ messages in thread
From: James Zern @ 2023-01-17 22:06 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

On Tue, Jan 10, 2023 at 5:23 PM James Zern <jzern@google.com> wrote:
>
> On Tue, Jan 10, 2023 at 2:47 AM mypopy@gmail.com <mypopy@gmail.com> wrote:
> >
> > On Thu, Jan 5, 2023 at 6:42 PM Dmitrii Ovchinnikov
> > <ovchinnikov.dmitrii@gmail.com> wrote:
> > [...]
> > > diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> > > index 9aa5510c28..0627e13973 100644
> > > --- a/libavcodec/libvpxenc.c
> > > +++ b/libavcodec/libvpxenc.c
> > > @@ -942,7 +942,7 @@ static av_cold int vpx_init(AVCodecContext *avctx,
> > >      enccfg.g_timebase.num = avctx->time_base.num;
> > >      enccfg.g_timebase.den = avctx->time_base.den;
> > >      enccfg.g_threads      =
> > > -        FFMIN(avctx->thread_count ? avctx->thread_count : av_cpu_count(), 16);
> > > +        avctx->thread_count ? avctx->thread_count : av_cpu_count();
> > >      enccfg.g_lag_in_frames= ctx->lag_in_frames;
> > >
> > Do you check the change with the old version libvpx?  as I know, older
> > versions libvpx setting the number of threads higher than 16 will
> > cause a crash, so I think at least a version check needs to be added
> >
>
> Do you have a bug or version in mind? There were some performance
> regressions [1] over the releases and some issues with changing the
> number of tiles, but I don't remember a crash for a high thread count
> (though there have been plenty of crashes related to threads in
> general [2]). The range check predates 1.4.0, which is the minimum
> required by ffmpeg.
>

I merged the changes without a check. If there is a problem with a
specific version we can add a check.

> [1] https://crbug.com/webm/1574
> [2] https://crbug.com/webm/851
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-01-17 22:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-05 10:41 [FFmpeg-devel] [PATCH v3] lavc/libvpx: remove thread limit Dmitrii Ovchinnikov
2023-01-05 17:14 ` Tomas Härdin
2023-01-10  3:06 ` James Zern
2023-01-10 10:47 ` mypopy
2023-01-11  1:23   ` James Zern
2023-01-17 22:06     ` James Zern

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git