On Sat, Feb 20, 2021 at 3:20 AM Christopher Degawa <c...@randomderp.com> wrote: > > From: Christopher Degawa <christopher.deg...@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 limit the > 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.deg...@intel.com> > --- > doc/encoders.texi | 5 +++++ > libavcodec/libsvtav1.c | 7 +++++++ > 2 files changed, 12 insertions(+) > > diff --git a/doc/encoders.texi b/doc/encoders.texi > index 8fb573c416..28c1a11a6c 100644 > --- a/doc/encoders.texi > +++ b/doc/encoders.texi > @@ -1757,6 +1757,11 @@ 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 limit threads. > +
Hi, and sorry for getting to this patch so slowly. Thank you for your discussion on IRC, I think that cleared up what this option does a bit. I think it might be more clear to call this just a "thread count multiplier override to limit the amount of threads utilized" in the docs and the help string, since that's what it essentially seems to boil to. According to what I grasped from your explanation, the encoder spawns XYZ threads per logical processor (core) by default, and this is a way to limit that calculation to a specific number. As SVT-AV1 does not have a separate switch to just plain set the amount of threads in general, it is also the only way right now to limit the thread count at the moment. ref: https://gitlab.com/AOMediaCodec/SVT-AV1/-/blob/05ed7cb78620c2ebbc948b20f26dd9a9c1756fa5/Source/Lib/Encoder/Globals/EbEncHandle.c#L254-257 > @end table > > @section libkvazaar > diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c > index eb6043bcac..2296735f25 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; > + Do we already require a new enough SVT-AV1 to always have this option? If yes, great. If not, it might be OK to just bump the requirement then (to keep unnecessary ifdefs at bay for a relatively new and actively developed library)? > 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, threads are managed by the OS scheduler", OFFSET(logical_processors), > + AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > + I think this could just be made to mention that it's a thread count multiplier override to limit the amount of threads utilized. There is an int/unsigned mismatch, but I think that should be OK since you limit the value to 0-INT_MAX in the AVOption itself? Otherwise LGTM, and once again sorry for taking the time to get to this. Jan _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
