[FFmpeg-devel] [PATCH v2] libsvtav1: Add logical_processors option

[Christopher Degawa]

On Tue, Mar 30, 2021 at 4:53 PM Jan Ekström <jeebjp at gmail.com> wrote:

> > @@ -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)?
>

afaik, this option has existed before the ffmpeg wrapper was upstreamed, so
I do not think any ifdefs are needed

ref:
https://gitlab.com/AOMediaCodec/SVT-AV1/-/commit/a6c1f81989c6cab0f477adfa867d5ff3dad2725c


> > +    { "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.
>

after discussing this option on IRC, I think the wording of thread count
multiplier would probably fit better, something along the lines of

> { "logical_processors", "Thread count multiplier with a max of the number
of logical cores available",

I will resubmit an updated patch soon


> 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?
>

changed to

> AV_OPT_TYPE_INT, { .i64 = 0 }, 0,  UINT_MAX, VE },


>
> Otherwise LGTM, and once again sorry for taking the time to get to this.
>
Thank you