| Message ID | 68088a70-85b7-1512-a326-9c75e49895cc@genshiken.org |
|---|---|
| State | New |
| Headers | show |
On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: > These options are required for multithreaded encoding, because they set > to zero by default in av1_cx_iface.c. > > Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> > > diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > index 9431179886..55cb7ff72e 100644 > --- a/libavcodec/libaomenc.c > +++ b/libavcodec/libaomenc.c > @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { > int static_thresh; > int drop_threshold; > int noise_sensitivity; > + int tile_columns; > + int tile_rows; > } AOMContext; > > static const char *const ctlidstr[] = { > @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { > [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", > [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", > [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", > + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", > + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", > [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", > [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", > [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", > @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, > if (ctx->crf >= 0) > codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); > > + if (ctx->tile_columns >= 0) > + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); > + if (ctx->tile_rows >= 0) > + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); > + > codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); > codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); > codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); > @@ -746,6 +755,8 @@ static const AVOption options[] = { > { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, > { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, > + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, Why 6? The libaom API doesn't mention a limit, just says the argument should be in log2 unit, and that it will be capped based on the image size. > + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, And for this one it says the range is [0-2]. > { NULL } > }; > >
On 31/08/18 02:58, James Almer wrote: > On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: >> These options are required for multithreaded encoding, because they set >> to zero by default in av1_cx_iface.c. >> >> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> >> >> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >> index 9431179886..55cb7ff72e 100644 >> --- a/libavcodec/libaomenc.c >> +++ b/libavcodec/libaomenc.c >> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { >> int static_thresh; >> int drop_threshold; >> int noise_sensitivity; >> + int tile_columns; >> + int tile_rows; >> } AOMContext; >> >> static const char *const ctlidstr[] = { >> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { >> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", >> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", >> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", >> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", >> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", >> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", >> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", >> [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", >> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, >> if (ctx->crf >= 0) >> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); >> >> + if (ctx->tile_columns >= 0) >> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); >> + if (ctx->tile_rows >= 0) >> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); >> + >> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); >> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); >> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); >> @@ -746,6 +755,8 @@ static const AVOption options[] = { >> { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, >> { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, >> { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, >> + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > Why 6? The libaom API doesn't mention a limit, just says the argument > should be in log2 unit, and that it will be capped based on the image size. https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 >> + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > And for this one it says the range is [0-2]. Who says it? I can pass --tile-rows=6 to aomenc.
On 8/31/2018 11:52 AM, Kagami Hiiragi wrote: > On 31/08/18 02:58, James Almer wrote: >> On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: >>> These options are required for multithreaded encoding, because they set >>> to zero by default in av1_cx_iface.c. >>> >>> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> >>> >>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >>> index 9431179886..55cb7ff72e 100644 >>> --- a/libavcodec/libaomenc.c >>> +++ b/libavcodec/libaomenc.c >>> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { >>> int static_thresh; >>> int drop_threshold; >>> int noise_sensitivity; >>> + int tile_columns; >>> + int tile_rows; >>> } AOMContext; >>> >>> static const char *const ctlidstr[] = { >>> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { >>> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", >>> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", >>> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", >>> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", >>> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", >>> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", >>> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", >>> [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", >>> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, >>> if (ctx->crf >= 0) >>> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); >>> >>> + if (ctx->tile_columns >= 0) >>> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); >>> + if (ctx->tile_rows >= 0) >>> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); >>> + >>> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); >>> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); >>> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); >>> @@ -746,6 +755,8 @@ static const AVOption options[] = { >>> { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, >>> { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, >>> { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, >>> + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >> >> Why 6? The libaom API doesn't mention a limit, just says the argument >> should be in log2 unit, and that it will be capped based on the image size. > > https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 > >>> + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >> >> And for this one it says the range is [0-2]. > > Who says it? I can pass --tile-rows=6 to aomenc. The doxy in the public header: https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 One shouldn't have to look at source code when there's documentation for public API, but since the latter is apparently wrong...
On 31/08/2018 16:18, James Almer wrote: > The doxy in the public header: > https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 > > One shouldn't have to look at source code when there's documentation for > public API, but since the latter is apparently wrong... If it's like this, at the very least, it should be noted where this magic number came from in libaom.c. - Derek
On 31/08/18 18:18, James Almer wrote: > On 8/31/2018 11:52 AM, Kagami Hiiragi wrote: >> On 31/08/18 02:58, James Almer wrote: >>> On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: >>>> These options are required for multithreaded encoding, because they set >>>> to zero by default in av1_cx_iface.c. >>>> >>>> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> >>>> >>>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >>>> index 9431179886..55cb7ff72e 100644 >>>> --- a/libavcodec/libaomenc.c >>>> +++ b/libavcodec/libaomenc.c >>>> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { >>>> int static_thresh; >>>> int drop_threshold; >>>> int noise_sensitivity; >>>> + int tile_columns; >>>> + int tile_rows; >>>> } AOMContext; >>>> >>>> static const char *const ctlidstr[] = { >>>> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { >>>> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", >>>> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", >>>> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", >>>> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", >>>> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", >>>> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", >>>> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", >>>> [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", >>>> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, >>>> if (ctx->crf >= 0) >>>> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); >>>> >>>> + if (ctx->tile_columns >= 0) >>>> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); >>>> + if (ctx->tile_rows >= 0) >>>> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); >>>> + >>>> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); >>>> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); >>>> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); >>>> @@ -746,6 +755,8 @@ static const AVOption options[] = { >>>> { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, >>>> { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, >>>> { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, >>>> + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >>> >>> Why 6? The libaom API doesn't mention a limit, just says the argument >>> should be in log2 unit, and that it will be capped based on the image size. >> >> https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 >> >>>> + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >>> >>> And for this one it says the range is [0-2]. >> >> Who says it? I can pass --tile-rows=6 to aomenc. > > The doxy in the public header: > https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 I guess it wasn't fixed after https://aomedia.googlesource.com/aom/+/492c545fad1e1f980d23d79d0372857c60d31458^!/#F1 I don't think ffmpeg should deal with libaom documentation issues...
On 8/31/2018 1:00 PM, Kagami Hiiragi wrote: > On 31/08/18 18:18, James Almer wrote: >> On 8/31/2018 11:52 AM, Kagami Hiiragi wrote: >>> On 31/08/18 02:58, James Almer wrote: >>>> On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: >>>>> These options are required for multithreaded encoding, because they set >>>>> to zero by default in av1_cx_iface.c. >>>>> >>>>> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> >>>>> >>>>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c >>>>> index 9431179886..55cb7ff72e 100644 >>>>> --- a/libavcodec/libaomenc.c >>>>> +++ b/libavcodec/libaomenc.c >>>>> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { >>>>> int static_thresh; >>>>> int drop_threshold; >>>>> int noise_sensitivity; >>>>> + int tile_columns; >>>>> + int tile_rows; >>>>> } AOMContext; >>>>> >>>>> static const char *const ctlidstr[] = { >>>>> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { >>>>> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", >>>>> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", >>>>> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", >>>>> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", >>>>> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", >>>>> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", >>>>> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", >>>>> [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", >>>>> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, >>>>> if (ctx->crf >= 0) >>>>> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); >>>>> >>>>> + if (ctx->tile_columns >= 0) >>>>> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); >>>>> + if (ctx->tile_rows >= 0) >>>>> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); >>>>> + >>>>> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); >>>>> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); >>>>> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); >>>>> @@ -746,6 +755,8 @@ static const AVOption options[] = { >>>>> { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, >>>>> { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, >>>>> { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, >>>>> + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >>>> >>>> Why 6? The libaom API doesn't mention a limit, just says the argument >>>> should be in log2 unit, and that it will be capped based on the image size. >>> >>> https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 >>> >>>>> + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, >>>> >>>> And for this one it says the range is [0-2]. >>> >>> Who says it? I can pass --tile-rows=6 to aomenc. >> >> The doxy in the public header: >> https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 > > I guess it wasn't fixed after > https://aomedia.googlesource.com/aom/+/492c545fad1e1f980d23d79d0372857c60d31458^!/#F1 > > I don't think ffmpeg should deal with libaom documentation issues... Seeing it's effectively a documentation issue, and that the change you quoted was made before libaom 1.0.0 was tagged, then i guess the patch is ok. I reported it to libaom bug tracker as well.
On Fri, Aug 31, 2018 at 9:40 AM James Almer <jamrial@gmail.com> wrote: > > On 8/31/2018 1:00 PM, Kagami Hiiragi wrote: > > On 31/08/18 18:18, James Almer wrote: > >> On 8/31/2018 11:52 AM, Kagami Hiiragi wrote: > >>> On 31/08/18 02:58, James Almer wrote: > >>>> On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: > >>>>> These options are required for multithreaded encoding, because they set > >>>>> to zero by default in av1_cx_iface.c. > >>>>> > >>>>> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> > >>>>> > >>>>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > >>>>> index 9431179886..55cb7ff72e 100644 > >>>>> --- a/libavcodec/libaomenc.c > >>>>> +++ b/libavcodec/libaomenc.c > >>>>> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { > >>>>> int static_thresh; > >>>>> int drop_threshold; > >>>>> int noise_sensitivity; > >>>>> + int tile_columns; > >>>>> + int tile_rows; > >>>>> } AOMContext; > >>>>> > >>>>> static const char *const ctlidstr[] = { > >>>>> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { > >>>>> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", > >>>>> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", > >>>>> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", > >>>>> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", > >>>>> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", > >>>>> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", > >>>>> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", > >>>>> [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", > >>>>> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, > >>>>> if (ctx->crf >= 0) > >>>>> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); > >>>>> > >>>>> + if (ctx->tile_columns >= 0) > >>>>> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); > >>>>> + if (ctx->tile_rows >= 0) > >>>>> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); > >>>>> + > >>>>> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); > >>>>> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); > >>>>> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); > >>>>> @@ -746,6 +755,8 @@ static const AVOption options[] = { > >>>>> { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > >>>>> { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, > >>>>> { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, > >>>>> + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > >>>> > >>>> Why 6? The libaom API doesn't mention a limit, just says the argument > >>>> should be in log2 unit, and that it will be capped based on the image size. > >>> > >>> https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 > >>> > >>>>> + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > >>>> > >>>> And for this one it says the range is [0-2]. > >>> > >>> Who says it? I can pass --tile-rows=6 to aomenc. > >> > >> The doxy in the public header: > >> https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 > > > > I guess it wasn't fixed after > > https://aomedia.googlesource.com/aom/+/492c545fad1e1f980d23d79d0372857c60d31458^!/#F1 > > > > I don't think ffmpeg should deal with libaom documentation issues... > > Seeing it's effectively a documentation issue, and that the change you > quoted was made before libaom 1.0.0 was tagged, then i guess the patch > is ok. > > I reported it to libaom bug tracker as well. Thanks for filing the bug, I posted a fix. Note AV1 supports non-uniform tiles now and there's another way to specify absolute widths and heights (--tile-width/height). This parameter wasn't descaled to allow the encoder to choose the layout unfortunately [1]. [1] https://bugs.chromium.org/p/aomedia/issues/detail?id=2124
Hi, Max value for both the tile rows and tile columns is 64 (so the max log2 value would be 6). This is consistent in the public header: https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#291 and spec: https://aomediacodec.github.io/av1-spec/av1-spec.pdf (see page 7). On Fri, Aug 31, 2018 at 6:08 PM James Zern <jzern-at-google.com@ffmpeg.org> wrote: > On Fri, Aug 31, 2018 at 9:40 AM James Almer <jamrial@gmail.com> wrote: > > > > On 8/31/2018 1:00 PM, Kagami Hiiragi wrote: > > > On 31/08/18 18:18, James Almer wrote: > > >> On 8/31/2018 11:52 AM, Kagami Hiiragi wrote: > > >>> On 31/08/18 02:58, James Almer wrote: > > >>>> On 8/20/2018 2:56 PM, Kagami Hiiragi wrote: > > >>>>> These options are required for multithreaded encoding, because > they set > > >>>>> to zero by default in av1_cx_iface.c. > > >>>>> > > >>>>> Signed-off-by: Kagami Hiiragi <kagami@genshiken.org> > > >>>>> > > >>>>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c > > >>>>> index 9431179886..55cb7ff72e 100644 > > >>>>> --- a/libavcodec/libaomenc.c > > >>>>> +++ b/libavcodec/libaomenc.c > > >>>>> @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { > > >>>>> int static_thresh; > > >>>>> int drop_threshold; > > >>>>> int noise_sensitivity; > > >>>>> + int tile_columns; > > >>>>> + int tile_rows; > > >>>>> } AOMContext; > > >>>>> > > >>>>> static const char *const ctlidstr[] = { > > >>>>> @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { > > >>>>> [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", > > >>>>> [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", > > >>>>> [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", > > >>>>> + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", > > >>>>> + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", > > >>>>> [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", > > >>>>> [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", > > >>>>> [AV1E_SET_MATRIX_COEFFICIENTS] = > "AV1E_SET_MATRIX_COEFFICIENTS", > > >>>>> @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext > *avctx, > > >>>>> if (ctx->crf >= 0) > > >>>>> codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); > > >>>>> > > >>>>> + if (ctx->tile_columns >= 0) > > >>>>> + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, > ctx->tile_columns); > > >>>>> + if (ctx->tile_rows >= 0) > > >>>>> + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); > > >>>>> + > > >>>>> codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, > avctx->color_primaries); > > >>>>> codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, > avctx->colorspace); > > >>>>> codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, > avctx->color_trc); > > >>>>> @@ -746,6 +755,8 @@ static const AVOption options[] = { > > >>>>> { "static-thresh", "A change threshold on blocks below > which they will be skipped by the encoder", OFFSET(static_thresh), > AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, > > >>>>> { "drop-threshold", "Frame drop threshold", > offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, > INT_MIN, INT_MAX, VE }, > > >>>>> { "noise-sensitivity", "Noise sensitivity", > OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, > > >>>>> + { "tile-columns", "Number of tile columns to use, log2", > OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > >>>> > > >>>> Why 6? The libaom API doesn't mention a limit, just says the > argument > > >>>> should be in log2 unit, and that it will be capped based on the > image size. > > >>> > > >>> > https://aomedia.googlesource.com/aom/+/1742b043e2269dc1927afe428fbf5a46493e741e/av1/av1_cx_iface.c#298 > > >>> > > >>>>> + { "tile-rows", "Number of tile rows to use, log2", > OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, > > >>>> > > >>>> And for this one it says the range is [0-2]. > > >>> > > >>> Who says it? I can pass --tile-rows=6 to aomenc. > > >> > > >> The doxy in the public header: > > >> https://aomedia.googlesource.com/aom/+/master/aom/aomcx.h#312 > > > > > > I guess it wasn't fixed after > > > > https://aomedia.googlesource.com/aom/+/492c545fad1e1f980d23d79d0372857c60d31458 > ^!/#F1 > > > > > > I don't think ffmpeg should deal with libaom documentation issues... > > > > Seeing it's effectively a documentation issue, and that the change you > > quoted was made before libaom 1.0.0 was tagged, then i guess the patch > > is ok. > > > > I reported it to libaom bug tracker as well. > > Thanks for filing the bug, I posted a fix. Note AV1 supports > non-uniform tiles now and there's another way to specify absolute > widths and heights (--tile-width/height). This parameter wasn't > descaled to allow the encoder to choose the layout unfortunately [1]. > > [1] https://bugs.chromium.org/p/aomedia/issues/detail?id=2124 > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c index 9431179886..55cb7ff72e 100644 --- a/libavcodec/libaomenc.c +++ b/libavcodec/libaomenc.c @@ -68,6 +68,8 @@ typedef struct AOMEncoderContext { int static_thresh; int drop_threshold; int noise_sensitivity; + int tile_columns; + int tile_rows; } AOMContext; static const char *const ctlidstr[] = { @@ -75,6 +77,8 @@ static const char *const ctlidstr[] = { [AOME_SET_CQ_LEVEL] = "AOME_SET_CQ_LEVEL", [AOME_SET_ENABLEAUTOALTREF] = "AOME_SET_ENABLEAUTOALTREF", [AOME_SET_STATIC_THRESHOLD] = "AOME_SET_STATIC_THRESHOLD", + [AV1E_SET_TILE_COLUMNS] = "AV1E_SET_TILE_COLUMNS", + [AV1E_SET_TILE_ROWS] = "AV1E_SET_TILE_ROWS", [AV1E_SET_COLOR_RANGE] = "AV1E_SET_COLOR_RANGE", [AV1E_SET_COLOR_PRIMARIES] = "AV1E_SET_COLOR_PRIMARIES", [AV1E_SET_MATRIX_COEFFICIENTS] = "AV1E_SET_MATRIX_COEFFICIENTS", @@ -449,6 +453,11 @@ static av_cold int aom_init(AVCodecContext *avctx, if (ctx->crf >= 0) codecctl_int(avctx, AOME_SET_CQ_LEVEL, ctx->crf); + if (ctx->tile_columns >= 0) + codecctl_int(avctx, AV1E_SET_TILE_COLUMNS, ctx->tile_columns); + if (ctx->tile_rows >= 0) + codecctl_int(avctx, AV1E_SET_TILE_ROWS, ctx->tile_rows); + codecctl_int(avctx, AV1E_SET_COLOR_PRIMARIES, avctx->color_primaries); codecctl_int(avctx, AV1E_SET_MATRIX_COEFFICIENTS, avctx->colorspace); codecctl_int(avctx, AV1E_SET_TRANSFER_CHARACTERISTICS, avctx->color_trc); @@ -746,6 +755,8 @@ static const AVOption options[] = { { "static-thresh", "A change threshold on blocks below which they will be skipped by the encoder", OFFSET(static_thresh), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, VE }, { "drop-threshold", "Frame drop threshold", offsetof(AOMContext, drop_threshold), AV_OPT_TYPE_INT, {.i64 = 0 }, INT_MIN, INT_MAX, VE }, { "noise-sensitivity", "Noise sensitivity", OFFSET(noise_sensitivity), AV_OPT_TYPE_INT, {.i64 = 0 }, 0, 4, VE}, + { "tile-columns", "Number of tile columns to use, log2", OFFSET(tile_columns), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, + { "tile-rows", "Number of tile rows to use, log2", OFFSET(tile_rows), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 6, VE}, { NULL } };
These options are required for multithreaded encoding, because they set to zero by default in av1_cx_iface.c. Signed-off-by: Kagami Hiiragi <kagami@genshiken.org>