diff mbox series

[FFmpeg-devel,v2] avcodec/libvpxenc: Apply codec options to alpha codec context

Message ID 20210903083206.1152088-1-chelminski.adam@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] avcodec/libvpxenc: Apply codec options to alpha codec context | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Adam Chelminski Sept. 3, 2021, 8:32 a.m. UTC 
When encoding yuva420 (alpha) frames, the vpx encoder uses a second
vpx_codec_ctx to encode the alpha stream. However, codec options were
only being applied to the primary encoder. This patch updates
codecctl_int and codecctl_intp to also apply codec options to the alpha
codec context when encoding frames with alpha.

This is necessary to take advantage of libvpx speed optimizations
such as 'row-mt' and 'cpu-used' when encoding videos with alpha.
Without this patch, the speed optimizations are only applied to the
primary stream encoding, and the overall encoding is just as slow
as it would be without the options specified.

Signed-off-by: Adam Chelminski <chelminski.adam@gmail.com>
---
 doc/APIchanges         |  4 ++++
 libavcodec/libvpxenc.c | 26 ++++++++++++++++++++++++--
 libavcodec/version.h   |  2 +-
 3 files changed, 29 insertions(+), 3 deletions(-)

Comments

James Zern Sept. 4, 2021, 12:02 a.m. UTC | #1 
On Fri, Sep 3, 2021 at 1:33 AM Adam Chelminski
<chelminski.adam@gmail.com> wrote:
>
> When encoding yuva420 (alpha) frames, the vpx encoder uses a second
> vpx_codec_ctx to encode the alpha stream. However, codec options were
> only being applied to the primary encoder. This patch updates
> codecctl_int and codecctl_intp to also apply codec options to the alpha
> codec context when encoding frames with alpha.
>
> This is necessary to take advantage of libvpx speed optimizations
> such as 'row-mt' and 'cpu-used' when encoding videos with alpha.
> Without this patch, the speed optimizations are only applied to the
> primary stream encoding, and the overall encoding is just as slow
> as it would be without the options specified.
>
> Signed-off-by: Adam Chelminski <chelminski.adam@gmail.com>
> ---
>  doc/APIchanges         |  4 ++++
>  libavcodec/libvpxenc.c | 26 ++++++++++++++++++++++++--
>  libavcodec/version.h   |  2 +-
>  3 files changed, 29 insertions(+), 3 deletions(-)
>

lgtm. One could argue that the alpha encoder should have its own set
of options to allow for a higher quality alpha plane, but I think
aligning the configurations makes sense given the current state of the
code. I'll submit this next week if there aren't any comments.

> diff --git a/doc/APIchanges b/doc/APIchanges
> index d58272ebd4..67603162a2 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -14,6 +14,10 @@ libavutil:     2021-04-27
>
>  API changes, most recent first:
>
> +2021-09-03 - xxxxxxxxxx - lavc 59.7.101 - avcodec.h
> +  The libvpx encoder now passes command-line encoder options to the
> +  alpha stream encoder in addition to the primary stream encoder.
> +

This isn't an API change and is a little minor for the Changelog;
bumping the micro version should be enough for history tracking. I've
removed this locally.

>  2021-09-02 - xxxxxxxxxx - lavc 59.7.100 - avcodec.h
>    Incremented the number of elements of AVCodecParser.codec_ids to seven.
>
> diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
> index 0e50fbfd7c..f66345e998 100644
> --- a/libavcodec/libvpxenc.c
> +++ b/libavcodec/libvpxenc.c
> @@ -385,9 +385,20 @@ static av_cold int codecctl_int(AVCodecContext *avctx,
>          snprintf(buf, sizeof(buf), "Failed to set %s codec control",
>                   ctlidstr[id]);
>          log_encoder_error(avctx, buf);
> +        return AVERROR(EINVAL);
>      }
>
> -    return res == VPX_CODEC_OK ? 0 : AVERROR(EINVAL);
> +    if (ctx->is_alpha) {
> +        int res_alpha = vpx_codec_control(&ctx->encoder_alpha, id, val);
> +        if (res_alpha != VPX_CODEC_OK) {
> +            snprintf(buf, sizeof(buf), "Failed to set %s alpha codec control",
> +                     ctlidstr[id]);
> +            log_encoder_error(avctx, buf);
> +            return AVERROR(EINVAL);
> +        }
> +    }
> +
> +    return 0;
>  }
>
>  #if VPX_ENCODER_ABI_VERSION >= 12
> @@ -407,9 +418,20 @@ static av_cold int codecctl_intp(AVCodecContext *avctx,
>          snprintf(buf, sizeof(buf), "Failed to set %s codec control",
>                   ctlidstr[id]);
>          log_encoder_error(avctx, buf);
> +        return AVERROR(EINVAL);
>      }
>
> -    return res == VPX_CODEC_OK ? 0 : AVERROR(EINVAL);
> +    if (ctx->is_alpha) {
> +        int res_alpha = vpx_codec_control(&ctx->encoder_alpha, id, val);
> +        if (res_alpha != VPX_CODEC_OK) {
> +            snprintf(buf, sizeof(buf), "Failed to set %s alpha codec control",
> +                     ctlidstr[id]);
> +            log_encoder_error(avctx, buf);
> +            return AVERROR(EINVAL);
> +        }
> +    }
> +
> +    return 0;
>  }
>  #endif
>
> diff --git a/libavcodec/version.h b/libavcodec/version.h
> index 7faf18a497..7839bf945d 100644
> --- a/libavcodec/version.h
> +++ b/libavcodec/version.h
> @@ -29,7 +29,7 @@
>
>  #define LIBAVCODEC_VERSION_MAJOR  59
>  #define LIBAVCODEC_VERSION_MINOR   7
> -#define LIBAVCODEC_VERSION_MICRO 100
> +#define LIBAVCODEC_VERSION_MICRO 101
>
>  #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
>                                                 LIBAVCODEC_VERSION_MINOR, \
> --
> 2.25.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
James Zern Sept. 9, 2021, 2:05 a.m. UTC | #2 
On Fri, Sep 3, 2021 at 5:02 PM James Zern <jzern@google.com> wrote:
>
> On Fri, Sep 3, 2021 at 1:33 AM Adam Chelminski
> <chelminski.adam@gmail.com> wrote:
> >
> > When encoding yuva420 (alpha) frames, the vpx encoder uses a second
> > vpx_codec_ctx to encode the alpha stream. However, codec options were
> > only being applied to the primary encoder. This patch updates
> > codecctl_int and codecctl_intp to also apply codec options to the alpha
> > codec context when encoding frames with alpha.
> >
> > This is necessary to take advantage of libvpx speed optimizations
> > such as 'row-mt' and 'cpu-used' when encoding videos with alpha.
> > Without this patch, the speed optimizations are only applied to the
> > primary stream encoding, and the overall encoding is just as slow
> > as it would be without the options specified.
> >
> > Signed-off-by: Adam Chelminski <chelminski.adam@gmail.com>
> > ---
> >  doc/APIchanges         |  4 ++++
> >  libavcodec/libvpxenc.c | 26 ++++++++++++++++++++++++--
> >  libavcodec/version.h   |  2 +-
> >  3 files changed, 29 insertions(+), 3 deletions(-)
> >
>
> lgtm. One could argue that the alpha encoder should have its own set
> of options to allow for a higher quality alpha plane, but I think
> aligning the configurations makes sense given the current state of the
> code. I'll submit this next week if there aren't any comments.
>

Applied. Thanks for the patch.
diff mbox series

Patch

diff --git a/doc/APIchanges b/doc/APIchanges
index d58272ebd4..67603162a2 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,10 @@  libavutil:     2021-04-27
 
 API changes, most recent first:
 
+2021-09-03 - xxxxxxxxxx - lavc 59.7.101 - avcodec.h
+  The libvpx encoder now passes command-line encoder options to the
+  alpha stream encoder in addition to the primary stream encoder.
+
 2021-09-02 - xxxxxxxxxx - lavc 59.7.100 - avcodec.h
   Incremented the number of elements of AVCodecParser.codec_ids to seven.
 
diff --git a/libavcodec/libvpxenc.c b/libavcodec/libvpxenc.c
index 0e50fbfd7c..f66345e998 100644
--- a/libavcodec/libvpxenc.c
+++ b/libavcodec/libvpxenc.c
@@ -385,9 +385,20 @@  static av_cold int codecctl_int(AVCodecContext *avctx,
         snprintf(buf, sizeof(buf), "Failed to set %s codec control",
                  ctlidstr[id]);
         log_encoder_error(avctx, buf);
+        return AVERROR(EINVAL);
     }
 
-    return res == VPX_CODEC_OK ? 0 : AVERROR(EINVAL);
+    if (ctx->is_alpha) {
+        int res_alpha = vpx_codec_control(&ctx->encoder_alpha, id, val);
+        if (res_alpha != VPX_CODEC_OK) {
+            snprintf(buf, sizeof(buf), "Failed to set %s alpha codec control",
+                     ctlidstr[id]);
+            log_encoder_error(avctx, buf);
+            return AVERROR(EINVAL);
+        }
+    }
+
+    return 0;
 }
 
 #if VPX_ENCODER_ABI_VERSION >= 12
@@ -407,9 +418,20 @@  static av_cold int codecctl_intp(AVCodecContext *avctx,
         snprintf(buf, sizeof(buf), "Failed to set %s codec control",
                  ctlidstr[id]);
         log_encoder_error(avctx, buf);
+        return AVERROR(EINVAL);
     }
 
-    return res == VPX_CODEC_OK ? 0 : AVERROR(EINVAL);
+    if (ctx->is_alpha) {
+        int res_alpha = vpx_codec_control(&ctx->encoder_alpha, id, val);
+        if (res_alpha != VPX_CODEC_OK) {
+            snprintf(buf, sizeof(buf), "Failed to set %s alpha codec control",
+                     ctlidstr[id]);
+            log_encoder_error(avctx, buf);
+            return AVERROR(EINVAL);
+        }
+    }
+
+    return 0;
 }
 #endif
 
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 7faf18a497..7839bf945d 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@ 
 
 #define LIBAVCODEC_VERSION_MAJOR  59
 #define LIBAVCODEC_VERSION_MINOR   7
-#define LIBAVCODEC_VERSION_MICRO 100
+#define LIBAVCODEC_VERSION_MICRO 101
 
 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \