diff mbox series

[FFmpeg-devel,v4] libsvtav1: Add logical_processors option

Message ID 20210310061234.1412193-1-ccom@randomderp.com
State New
Headers show
Series [FFmpeg-devel,v4] libsvtav1: Add logical_processors option | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Christopher Degawa March 10, 2021, 6:12 a.m. UTC 
From: Christopher Degawa <christopher.degawa@intel.com>

Used for limiting the size of memory buffers and threads for a target
logical processor count, but does not set thread affinity or the total
amount of threads used, although thread affinities can be controlled
with an additional parameters, it is prefered to add them until
a svtav1-params option or similar is added

Signed-off-by: Christopher Degawa <christopher.degawa@intel.com>
---
 doc/encoders.texi      | 6 ++++++
 libavcodec/libsvtav1.c | 7 +++++++
 2 files changed, 13 insertions(+)

--
2.25.1

Comments

Moritz Barsnick March 10, 2021, 5:29 p.m. UTC | #1 
On Wed, Mar 10, 2021 at 00:12:34 -0600, Christopher Degawa wrote:
> +@item logical_processors
> +Number of logical processors to run the encoder on, threads are managed by the OS scheduler.
> +Used for limiting the size of memory buffers and threads for a target logical processor count.
> +Does not set thread affinity or total threads, but instead sets t * logical_processors amount of threads
> +with t being the amount of threads libsvtav1 sets per cpu (0 - ncpus).

You should restrict the line length here, just for consistency with the
rest of this document. (It doesn't affect the formatting of the output
documents, obviously.)

> +    unsigned logical_processors;

The libsvtav1 API defines this as uint32_t, so I believe you should
mirror that.

> +      AV_OPT_TYPE_INT, { .i64 = 0 }, 0,  INT_MAX, VE },

Probably UINT_MAX, though I doubt that such a number of processors will
be reached. ;-) I don't know whether there's another natural limit
within libsvtav1.

Cheers,
Moritz
Christopher Degawa March 10, 2021, 5:49 p.m. UTC | #2 
> You should restrict the line length here, just for consistency with the
> rest of this document. (It doesn't affect the formatting of the output
> documents, obviously.)
>
What would be a good line length for the docs to aim for? 75?


> > +    unsigned logical_processors;
>
> The libsvtav1 API defines this as uint32_t, so I believe you should
> mirror that.
>
Changed locally, didn't see any other usage of stdint.h types in the file
so I just followed along



> > +      AV_OPT_TYPE_INT, { .i64 = 0 }, 0,  INT_MAX, VE },
>
> Probably UINT_MAX, though I doubt that such a number of processors will
> be reached. ;-) I don't know whether there's another natural limit
> within libsvtav1.
>
Changed locally, hopefully, one day this limit will actually have a use
(crossed fingers for AMD or some potentially arm or riscv).

The only internal limits I am aware of is the return type of sysconf being
shortened to uint32_t and windows' DWORD length

Thanks, Chris
diff mbox series

Patch

diff --git a/doc/encoders.texi b/doc/encoders.texi
index c9c8785afb..7bb97d9dae 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1795,6 +1795,12 @@  Set log2 of the number of rows of tiles to use (0-6).
 @item tile_columns
 Set log2 of the number of columns of tiles to use (0-4).

+@item logical_processors
+Number of logical processors to run the encoder on, threads are managed by the OS scheduler.
+Used for limiting the size of memory buffers and threads for a target logical processor count.
+Does not set thread affinity or total threads, but instead sets t * logical_processors amount of threads
+with t being the amount of threads libsvtav1 sets per cpu (0 - ncpus).
+
 @end table

 @section libkvazaar
diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index eb6043bcac..087b14099f 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -71,6 +71,8 @@  typedef struct SvtContext {

     int tile_columns;
     int tile_rows;
+
+    unsigned logical_processors;
 } SvtContext;

 static const struct {
@@ -218,6 +220,8 @@  static int config_enc_params(EbSvtAv1EncConfiguration *param,
     param->tile_columns = svt_enc->tile_columns;
     param->tile_rows    = svt_enc->tile_rows;

+    param->logical_processors = svt_enc->logical_processors;
+
     return 0;
 }

@@ -533,6 +537,9 @@  static const AVOption options[] = {
     { "tile_columns", "Log2 of number of tile columns to use", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 4, VE},
     { "tile_rows", "Log2 of number of tile rows to use", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = 0}, 0, 6, VE},

+    { "logical_processors", "Number of logical processors to run the encoder on, used to limit the size of memory buffers and threads used", OFFSET(logical_processors),
+      AV_OPT_TYPE_INT, { .i64 = 0 }, 0,  INT_MAX, VE },
+
     {NULL},
 };