[FFmpeg-devel] avcodec/libaomenc: add row-mt option

Default to disable, same as aomenc.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/libaomenc.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

On 08/12/2018 23:18, James Almer wrote:
> Default to disable, same as aomenc.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/libaomenc.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 17565017b4..1d3cef73f3 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -78,6 +78,7 @@ typedef struct AOMEncoderContext {
>      int tile_cols_log2, tile_rows_log2;
>      aom_superblock_size_t superblock_size;
>      int uniform_tiles;
> +    int row_mt;
>  } AOMContext;
>  
>  static const char *const ctlidstr[] = {
> @@ -92,6 +93,9 @@ static const char *const ctlidstr[] = {
>      [AV1E_SET_SUPERBLOCK_SIZE]  = "AV1E_SET_SUPERBLOCK_SIZE",
>      [AV1E_SET_TILE_COLUMNS]     = "AV1E_SET_TILE_COLUMNS",
>      [AV1E_SET_TILE_ROWS]        = "AV1E_SET_TILE_ROWS",
> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
> +    [AV1E_SET_ROW_MT]           = "AV1E_SET_ROW_MT",
> +#endif
>  };
>  
>  static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
> @@ -650,6 +654,10 @@ static av_cold int aom_init(AVCodecContext *avctx,
>          codecctl_int(avctx, AV1E_SET_TILE_ROWS,    ctx->tile_rows_log2);
>      }
>  
> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
> +    codecctl_int(avctx, AV1E_SET_ROW_MT, ctx->row_mt);
> +#endif
> +
>      // provide dummy value to initialize wrapper, values will be updated each _encode()
>      aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
>                   (unsigned char*)1);
> @@ -983,6 +991,9 @@ static const AVOption options[] = {
>      { "tiles",            "Tile columns x rows", OFFSET(tile_cols), AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, VE },
>      { "tile-columns",     "Log2 of number of tile columns to use", OFFSET(tile_cols_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>      { "tile-rows",        "Log2 of number of tile rows to use",    OFFSET(tile_rows_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
> +    { "row-mt",           "Enable row based multi-threading",      OFFSET(row_mt),         AV_OPT_TYPE_BOOL, {.i64 = 0},  0, 1, VE},
> +#endif
>      { NULL }
>  };

With matching update to doc/encoders.texi, LGTM.

Is there any reason not to enable it by default when threads are enabled?  (E.g. because it can make the quality worse somehow?)

Thanks,

- Mark

On 12/9/2018 2:25 PM, Mark Thompson wrote:
> On 08/12/2018 23:18, James Almer wrote:
>> Default to disable, same as aomenc.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/libaomenc.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
>> index 17565017b4..1d3cef73f3 100644
>> --- a/libavcodec/libaomenc.c
>> +++ b/libavcodec/libaomenc.c
>> @@ -78,6 +78,7 @@ typedef struct AOMEncoderContext {
>>      int tile_cols_log2, tile_rows_log2;
>>      aom_superblock_size_t superblock_size;
>>      int uniform_tiles;
>> +    int row_mt;
>>  } AOMContext;
>>  
>>  static const char *const ctlidstr[] = {
>> @@ -92,6 +93,9 @@ static const char *const ctlidstr[] = {
>>      [AV1E_SET_SUPERBLOCK_SIZE]  = "AV1E_SET_SUPERBLOCK_SIZE",
>>      [AV1E_SET_TILE_COLUMNS]     = "AV1E_SET_TILE_COLUMNS",
>>      [AV1E_SET_TILE_ROWS]        = "AV1E_SET_TILE_ROWS",
>> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
>> +    [AV1E_SET_ROW_MT]           = "AV1E_SET_ROW_MT",
>> +#endif
>>  };
>>  
>>  static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
>> @@ -650,6 +654,10 @@ static av_cold int aom_init(AVCodecContext *avctx,
>>          codecctl_int(avctx, AV1E_SET_TILE_ROWS,    ctx->tile_rows_log2);
>>      }
>>  
>> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
>> +    codecctl_int(avctx, AV1E_SET_ROW_MT, ctx->row_mt);
>> +#endif
>> +
>>      // provide dummy value to initialize wrapper, values will be updated each _encode()
>>      aom_img_wrap(&ctx->rawimg, img_fmt, avctx->width, avctx->height, 1,
>>                   (unsigned char*)1);
>> @@ -983,6 +991,9 @@ static const AVOption options[] = {
>>      { "tiles",            "Tile columns x rows", OFFSET(tile_cols), AV_OPT_TYPE_IMAGE_SIZE, { .str = NULL }, 0, 0, VE },
>>      { "tile-columns",     "Log2 of number of tile columns to use", OFFSET(tile_cols_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>>      { "tile-rows",        "Log2 of number of tile rows to use",    OFFSET(tile_rows_log2), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE},
>> +#ifdef AOM_CTRL_AV1E_SET_ROW_MT
>> +    { "row-mt",           "Enable row based multi-threading",      OFFSET(row_mt),         AV_OPT_TYPE_BOOL, {.i64 = 0},  0, 1, VE},
>> +#endif
>>      { NULL }
>>  };
> 
> With matching update to doc/encoders.texi, LGTM.
> 
> Is there any reason not to enable it by default when threads are enabled?  (E.g. because it can make the quality worse somehow?)

I don't know, maybe it disables tile multi-threading if it's enabled?

I decided to use the same default value aomenc sets for it, since a
feature being disabled by default tends to hint that it's either not yet
mature, or that the cons outweigh the pros.

> 
> Thanks,
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>