Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/libaomenc.c: Add super-resolution options to libaom wrapper

2020-03-05 Thread Wang Cao 
On Wed, Mar 4, 2020 at 1:38 AM Moritz Barsnick  wrote:

> On Wed, Mar 04, 2020 at 05:59:03 +0800, Wang Cao wrote:
> > Signed-off-by: Wang Cao 
> > ---
> >  doc/encoders.texi  | 39 +++
> >  libavcodec/libaomenc.c | 38 ++
>
> Again, I suggest bumping MICRO once more.
>
Done.

> > +@item superres_denominator
> > +The denominator for superres to use when @option{superres-mode} is
> @option{fixed}. Valid value
> > +ranges from 8 to 16.
> > +
> > +@item superres_kf_denominator
> > +The denominator for superres to use on key frames is fixed when
> > +@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to
> 16.
>
> I believe "is fixed " is not needed in this sentence, or even
> confusing.
>
Done.

> >  [AOME_SET_TUNING]   = "AOME_SET_TUNING",
> > +[AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES",
>
> The other '=' in this block are aligned.
>
> > +if (ctx->superres_mode >= 0)
> > +  enccfg.rc_superres_mode = ctx->superres_mode;
> > +if (ctx->superres_qthresh > 0)
> > +  enccfg.rc_superres_qthresh = ctx->superres_qthresh;
> > +if (ctx->superres_kf_qthresh > 0)
> > +  enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh;
> > +if (ctx->superres_denominator >= 0)
> > +  enccfg.rc_superres_denominator = ctx->superres_denominator;
> > +if (ctx->superres_kf_denominator >= 0)
> > +  enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator;
>
> It looks like indentation is off - ffmpeg uses four spaces.
>
> (Is this struct zero-initialized / memset()'d?)

Yes it is initialized before through aom_codec_enc_config_default

>
>
>  { "ssim","SSIM as distortion metric", 0,
> AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "tune"},
> > +{ "enable-superres", "Enable super-resolution mode",
> OFFSET(enable_superres), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
> > +{ "superres-mode",   "Select super-resultion mode",
> OFFSET(superres_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 4, VE,
> "superres_mode"},
> > +{ "none","No frame superres allowed", 0,
> AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "superres_mode"},
> > +{ "fixed",   "All frames are coded at the specified scale
> and super-resolved", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE,
> "superres_mode"},
> > +{ "random",  "All frames are coded at a random scale and
> super-resolved.", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, 0, 0, VE,
> "superres_mode"},
> > +{ "qthresh", "Superres scale for a frame is determined
> based on q_index",  0, AV_OPT_TYPE_CONST, {.i64 = 3}, 0, 0, VE,
> "superres_mode"},
> > +{ "auto","Automatically select superres for appropriate
> frames",   0, AV_OPT_TYPE_CONST, {.i64 = 4}, 0, 0, VE,
> "superres_mode"},
>
> Several remarks:
> - The "none" entry should also be aligned, just like the entry "fixed"
>   and following... (starting at   "0, AV_OPT_TYPE_CONST", if you see
>   what I mean).
> - Is "auto" a value given by the library? I'm asking because
>   we tend to use "-1" for our internal "auto", and use that as a
>   default, if desired.
>   (From looking at libaom, they indeed use 4 for "auto".)
> - Can you directly use the enumerations provided as SUPERRES_MODE by
>   libaom, or are they not public?
> - If you cannot reuse said enum (and its resulting range
>   [-1, SUPERRES_MODES - 1]), you should define your own as enum
>   SuperresModes { AOM_SUPERRES_MODE_NONE, AOM_SUPERRES_MODE_FIXED, ,
>   AOM_SUPERRES_MODE_NB }, use these in the options definition above,
>   and set the allowed range to [-1, AOM_SUPERRES_MODE_NB - 1].
>
> This is a good approach to improve the readability. Done.

> Cheers,
> Moritz
>
Please find the changes in the updated patch. Thanks!
-- 

Wang Cao |  Software Engineer |  wang...@google.com |  650-203-7807
___
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".

Re: [FFmpeg-devel] [PATCH 2/2] libavcodec/libaomenc.c: Add super-resolution options to libaom wrapper

2020-03-04 Thread Moritz Barsnick 
On Wed, Mar 04, 2020 at 05:59:03 +0800, Wang Cao wrote:
> Signed-off-by: Wang Cao 
> ---
>  doc/encoders.texi  | 39 +++
>  libavcodec/libaomenc.c | 38 ++

Again, I suggest bumping MICRO once more.

> +@item superres_denominator
> +The denominator for superres to use when @option{superres-mode} is 
> @option{fixed}. Valid value
> +ranges from 8 to 16.
> +
> +@item superres_kf_denominator
> +The denominator for superres to use on key frames is fixed when
> +@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to 16.

I believe "is fixed " is not needed in this sentence, or even
confusing.

>  [AOME_SET_TUNING]   = "AOME_SET_TUNING",
> +[AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES",

The other '=' in this block are aligned.

> +if (ctx->superres_mode >= 0)
> +  enccfg.rc_superres_mode = ctx->superres_mode;
> +if (ctx->superres_qthresh > 0)
> +  enccfg.rc_superres_qthresh = ctx->superres_qthresh;
> +if (ctx->superres_kf_qthresh > 0)
> +  enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh;
> +if (ctx->superres_denominator >= 0)
> +  enccfg.rc_superres_denominator = ctx->superres_denominator;
> +if (ctx->superres_kf_denominator >= 0)
> +  enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator;

It looks like indentation is off - ffmpeg uses four spaces.

(Is this struct zero-initialized / memset()'d?)

>  { "ssim","SSIM as distortion metric", 0, 
> AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "tune"},
> +{ "enable-superres", "Enable super-resolution mode", 
> OFFSET(enable_superres), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
> +{ "superres-mode",   "Select super-resultion mode", 
> OFFSET(superres_mode), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 4, VE, 
> "superres_mode"},
> +{ "none","No frame superres allowed", 0, 
> AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0, VE, "superres_mode"},
> +{ "fixed",   "All frames are coded at the specified scale and 
> super-resolved", 0, AV_OPT_TYPE_CONST, {.i64 = 1}, 0, 0, VE, "superres_mode"},
> +{ "random",  "All frames are coded at a random scale and 
> super-resolved.", 0, AV_OPT_TYPE_CONST, {.i64 = 2}, 0, 0, VE, 
> "superres_mode"},
> +{ "qthresh", "Superres scale for a frame is determined based on 
> q_index",  0, AV_OPT_TYPE_CONST, {.i64 = 3}, 0, 0, VE, "superres_mode"},
> +{ "auto","Automatically select superres for appropriate 
> frames",   0, AV_OPT_TYPE_CONST, {.i64 = 4}, 0, 0, VE, 
> "superres_mode"},

Several remarks:
- The "none" entry should also be aligned, just like the entry "fixed"
  and following... (starting at   "0, AV_OPT_TYPE_CONST", if you see
  what I mean).
- Is "auto" a value given by the library? I'm asking because
  we tend to use "-1" for our internal "auto", and use that as a
  default, if desired.
  (From looking at libaom, they indeed use 4 for "auto".)
- Can you directly use the enumerations provided as SUPERRES_MODE by
  libaom, or are they not public?
- If you cannot reuse said enum (and its resulting range
  [-1, SUPERRES_MODES - 1]), you should define your own as enum
  SuperresModes { AOM_SUPERRES_MODE_NONE, AOM_SUPERRES_MODE_FIXED, ,
  AOM_SUPERRES_MODE_NB }, use these in the options definition above,
  and set the allowed range to [-1, AOM_SUPERRES_MODE_NB - 1].

Cheers,
Moritz
___
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".

[FFmpeg-devel] [PATCH 2/2] libavcodec/libaomenc.c: Add super-resolution options to libaom wrapper

2020-03-03 Thread Wang Cao 
Signed-off-by: Wang Cao 
---
 doc/encoders.texi  | 39 +++
 libavcodec/libaomenc.c | 38 ++
 2 files changed, 77 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 4215f237bd..ac00c305d5 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1608,6 +1608,45 @@ Enable the use of global motion for block prediction. 
Default is true.
 Enable block copy mode for intra block prediction. This mode is
 useful for screen content. Default is true.
 
+@item enable-superres (@emph{boolean})
+Enable super-resolution during the encoding process.
+
+@item superres-mode (@emph{mode})
+Select super-resultion mode
+
+@table @option
+@item none (@emph{0})
+No frame superres allowed
+
+@item fixed (@emph{1})
+All frames are coded at the specified scale and super-resolved
+
+@item random (@emph{2})
+All frames are coded at a random scale and super-resolved.
+
+@item qthresh (@emph{3})
+Superres scale for a frame is determined based on q_index
+
+@item auto (@emph{4})
+Automatically select superres for appropriate frames
+@end table
+
+@item superres_denominator
+The denominator for superres to use when @option{superres-mode} is 
@option{fixed}. Valid value 
+ranges from 8 to 16.
+
+@item superres_kf_denominator
+The denominator for superres to use on key frames is fixed when 
+@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to 16.
+
+@item superres_qthresh
+The q level threshold after which superres is used when @option{superres-mode} 
is 
+@option{qthresh}. Valid value ranges from 1 to 63.
+
+@item superres_kf_qthresh
+The q level threshold after which superres is used for key frames when 
+@option{superres-mode} is @option{qthresh}. Valid value ranges from 1 to 63.
+
 @end table
 
 @section libkvazaar
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 7fd624c470..f1578db5e6 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -95,6 +95,12 @@ typedef struct AOMEncoderContext {
 int enable_restoration;
 int usage;
 int tune;
+int enable_superres;
+int superres_mode;
+int superres_denominator;
+int superres_qthresh;
+int superres_kf_denominator;
+int superres_kf_qthresh;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
@@ -134,6 +140,7 @@ static const char *const ctlidstr[] = {
 #endif
 [AV1E_SET_ENABLE_CDEF]  = "AV1E_SET_ENABLE_CDEF",
 [AOME_SET_TUNING]   = "AOME_SET_TUNING",
+[AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES",
 };
 
 static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
@@ -203,6 +210,13 @@ static av_cold void dump_enc_cfg(AVCodecContext *avctx,
width, "tile_width_count:",  cfg->tile_width_count,
width, "tile_height_count:", cfg->tile_height_count);
 av_log(avctx, level, "\n");
+av_log(avctx, level, "super resolution settings\n"
+ "%*s%u\n  %*s%u\n  %*s%u\n  %*s%u\n  %*s%u\n  ",
+   width, "rc_superres_mode:",  cfg->rc_superres_mode,
+   width, "rc_superres_denominator:",cfg->rc_superres_denominator,
+   width, "rc_superres_qthresh:", cfg->rc_superres_qthresh,
+   width, "rc_superres_kf_denominator:", 
cfg->rc_superres_kf_denominator,
+   width, "rc_superres_kf_qthresh:", cfg->rc_superres_kf_qthresh);
 }
 
 static void coded_frame_add(void *list, struct FrameListData *cx_frame)
@@ -545,6 +559,17 @@ static av_cold int aom_init(AVCodecContext *avctx,
 return AVERROR(EINVAL);
 }
 
+if (ctx->superres_mode >= 0)
+  enccfg.rc_superres_mode = ctx->superres_mode;
+if (ctx->superres_qthresh > 0)
+  enccfg.rc_superres_qthresh = ctx->superres_qthresh;
+if (ctx->superres_kf_qthresh > 0)
+  enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh;
+if (ctx->superres_denominator >= 0)
+  enccfg.rc_superres_denominator = ctx->superres_denominator;
+if (ctx->superres_kf_denominator >= 0)
+  enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator;
+
 dump_enc_cfg(avctx, &enccfg);
 
 enccfg.g_w= avctx->width;
@@ -687,6 +712,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
 // codec control failures are currently treated only as warnings
 av_log(avctx, AV_LOG_DEBUG, "aom_codec_control\n");
 codecctl_int(avctx, AOME_SET_CPUUSED, ctx->cpu_used);
+if (ctx->enable_superres >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_SUPERRES, ctx->enable_superres);
 if (ctx->auto_alt_ref >= 0)
 codecctl_int(avctx, AOME_SET_ENABLEAUTOALTREF, ctx->auto_alt_ref);
 if (ctx->arnr_max_frames >= 0)
@@ -1103,6 +1130,17 @@ static const AVOption options[] = {
 { "tune","The metric that encoder tunes for", OFFSET(tune), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, VE, "tune"},
 { "psnr","PSNR as distortion metric", 0, 
AV_OPT_TYPE_CONST, {.i64 = 0}, 0, 0,

[FFmpeg-devel] [PATCH 2/2] libavcodec/libaomenc.c: Add super-resolution options to libaom wrapper

2020-03-03 Thread Wang Cao 
From: Wang Cao 

Signed-off-by: Wang Cao 
---
 doc/encoders.texi  | 39 +++
 libavcodec/libaomenc.c | 38 ++
 2 files changed, 77 insertions(+)

diff --git a/doc/encoders.texi b/doc/encoders.texi
index 4215f237bd..ac00c305d5 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1608,6 +1608,45 @@ Enable the use of global motion for block prediction. 
Default is true.
 Enable block copy mode for intra block prediction. This mode is
 useful for screen content. Default is true.
 
+@item enable-superres (@emph{boolean})
+Enable super-resolution during the encoding process.
+
+@item superres-mode (@emph{mode})
+Select super-resultion mode
+
+@table @option
+@item none (@emph{0})
+No frame superres allowed
+
+@item fixed (@emph{1})
+All frames are coded at the specified scale and super-resolved
+
+@item random (@emph{2})
+All frames are coded at a random scale and super-resolved.
+
+@item qthresh (@emph{3})
+Superres scale for a frame is determined based on q_index
+
+@item auto (@emph{4})
+Automatically select superres for appropriate frames
+@end table
+
+@item superres_denominator
+The denominator for superres to use when @option{superres-mode} is 
@option{fixed}. Valid value 
+ranges from 8 to 16.
+
+@item superres_kf_denominator
+The denominator for superres to use on key frames is fixed when 
+@option{superres-mode} is @option{fixed}. Valid value ranges from 8 to 16.
+
+@item superres_qthresh
+The q level threshold after which superres is used when @option{superres-mode} 
is 
+@option{qthresh}. Valid value ranges from 1 to 63.
+
+@item superres_kf_qthresh
+The q level threshold after which superres is used for key frames when 
+@option{superres-mode} is @option{qthresh}. Valid value ranges from 1 to 63.
+
 @end table
 
 @section libkvazaar
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 7fd624c470..f1578db5e6 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -95,6 +95,12 @@ typedef struct AOMEncoderContext {
 int enable_restoration;
 int usage;
 int tune;
+int enable_superres;
+int superres_mode;
+int superres_denominator;
+int superres_qthresh;
+int superres_kf_denominator;
+int superres_kf_qthresh;
 } AOMContext;
 
 static const char *const ctlidstr[] = {
@@ -134,6 +140,7 @@ static const char *const ctlidstr[] = {
 #endif
 [AV1E_SET_ENABLE_CDEF]  = "AV1E_SET_ENABLE_CDEF",
 [AOME_SET_TUNING]   = "AOME_SET_TUNING",
+[AV1E_SET_ENABLE_SUPERRES] = "AV1E_SET_ENABLE_SUPERRES",
 };
 
 static av_cold void log_encoder_error(AVCodecContext *avctx, const char *desc)
@@ -203,6 +210,13 @@ static av_cold void dump_enc_cfg(AVCodecContext *avctx,
width, "tile_width_count:",  cfg->tile_width_count,
width, "tile_height_count:", cfg->tile_height_count);
 av_log(avctx, level, "\n");
+av_log(avctx, level, "super resolution settings\n"
+ "%*s%u\n  %*s%u\n  %*s%u\n  %*s%u\n  %*s%u\n  ",
+   width, "rc_superres_mode:",  cfg->rc_superres_mode,
+   width, "rc_superres_denominator:",cfg->rc_superres_denominator,
+   width, "rc_superres_qthresh:", cfg->rc_superres_qthresh,
+   width, "rc_superres_kf_denominator:", 
cfg->rc_superres_kf_denominator,
+   width, "rc_superres_kf_qthresh:", cfg->rc_superres_kf_qthresh);
 }
 
 static void coded_frame_add(void *list, struct FrameListData *cx_frame)
@@ -545,6 +559,17 @@ static av_cold int aom_init(AVCodecContext *avctx,
 return AVERROR(EINVAL);
 }
 
+if (ctx->superres_mode >= 0)
+  enccfg.rc_superres_mode = ctx->superres_mode;
+if (ctx->superres_qthresh > 0)
+  enccfg.rc_superres_qthresh = ctx->superres_qthresh;
+if (ctx->superres_kf_qthresh > 0)
+  enccfg.rc_superres_kf_qthresh = ctx->superres_kf_qthresh;
+if (ctx->superres_denominator >= 0)
+  enccfg.rc_superres_denominator = ctx->superres_denominator;
+if (ctx->superres_kf_denominator >= 0)
+  enccfg.rc_superres_kf_denominator = ctx->superres_kf_denominator;
+
 dump_enc_cfg(avctx, &enccfg);
 
 enccfg.g_w= avctx->width;
@@ -687,6 +712,8 @@ static av_cold int aom_init(AVCodecContext *avctx,
 // codec control failures are currently treated only as warnings
 av_log(avctx, AV_LOG_DEBUG, "aom_codec_control\n");
 codecctl_int(avctx, AOME_SET_CPUUSED, ctx->cpu_used);
+if (ctx->enable_superres >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_SUPERRES, ctx->enable_superres);
 if (ctx->auto_alt_ref >= 0)
 codecctl_int(avctx, AOME_SET_ENABLEAUTOALTREF, ctx->auto_alt_ref);
 if (ctx->arnr_max_frames >= 0)
@@ -1103,6 +1130,17 @@ static const AVOption options[] = {
 { "tune","The metric that encoder tunes for", OFFSET(tune), 
AV_OPT_TYPE_INT, {.i64 = -1}, -1, 1, VE, "tune"},
 { "psnr","PSNR as distortion metric", 0, 
AV_OPT_TYPE_CONST,