Discussion:
[FFmpeg-devel] [PATCH] avcodec/libaomenc: add row-mt option
James Almer
2018-12-08 23:18:09 UTC
Permalink
Default to disable, same as aomenc.

Signed-off-by: James Almer <***@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 }
};
--
2.19.2
Mark Thompson
2018-12-09 17:25:40 UTC
Permalink
Post by James Almer
Default to disable, same as aomenc.
---
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
James Almer
2018-12-09 17:47:37 UTC
Permalink
Post by Mark Thompson
Post by James Almer
Default to disable, same as aomenc.
---
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.
Post by Mark Thompson
Thanks,
- Mark
_______________________________________________
ffmpeg-devel mailing list
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Loading...