diff mbox series

[FFmpeg-devel,2/2] lavc/libx264: add mb_info option

Message ID ff9b0b22be35733a7e3931b222bd643d00e67174.camel@amazon.it
State New
Headers show
Series None | expand

Commit Message

Carotti, Elias June 21, 2023, 3:57 p.m. UTC
Hi all,
please find the second part of the patch set.
Best,
Elias




NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico

Comments

Carotti, Elias July 3, 2023, 3:55 p.m. UTC | #1
On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote:
> Hi all,
> please find the second part of the patch set.
> Best,
> Elias

Hi all,
please find the second part of the patch, updating libavcodec/libx264.c
to use the AVVideoHint side data. This should use the block_size field
to scan the AVVideoRect array.

Best,
Elias



NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
Anton Khirnov July 9, 2023, 11:07 a.m. UTC | #2
Quoting Carotti, Elias (2023-07-03 17:55:40)
> On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote:
> > Hi all,
> > please find the second part of the patch set.
> > Best,
> > Elias
> 
> Hi all,
> please find the second part of the patch, updating libavcodec/libx264.c
> to use the AVVideoHint side data. This should use the block_size field
> to scan the AVVideoRect array.
> 
> Best,
> Elias
> 
> 
> 
> NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
> 
> 
> 
> From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001
> From: Elias Carotti <eliascrt@amazon.it>
> Date: Tue, 20 Jun 2023 19:29:08 +0200
> Subject: [PATCH 2/2] lavc/libx264: add mb_info option
> 
> Pass the information about unchanged parts of the frame by means of the
> AVVideoHint side data.
> ---
>  Changelog            |  1 +
>  doc/APIchanges       |  3 ++
>  libavcodec/libx264.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h |  2 +-
>  4 files changed, 99 insertions(+), 1 deletion(-)
> 
> diff --git a/Changelog b/Changelog
> index 3876082844..70b0fe94a3 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -25,6 +25,7 @@ version <next>:
>  - Raw VVC bitstream parser, muxer and demuxer
>  - Bitstream filter for editing metadata in VVC streams
>  - Bitstream filter for converting VVC from MP4 to Annex B
> +- support for the P_SKIP hinting to speed up libx264 encoding
>  
>  version 6.0:
>  - Radiance HDR image support
> diff --git a/doc/APIchanges b/doc/APIchanges
> index bfe04556d2..89ff88af15 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2023-02-09
>  
>  API changes, most recent first:
>  
> +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c
> +  Add mb_info option.

Do we actually need the option? If the encoder's caller bothered with
adding the side data, then I'd think it should always be used, otherwise
why is it even there?
Carotti, Elias July 11, 2023, 10:46 a.m. UTC | #3
> From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001
> From: Elias Carotti <eliascrt@amazon.it>
> Date: Tue, 20 Jun 2023 19:29:08 +0200
> Subject: [PATCH 2/2] lavc/libx264: add mb_info option
>
> Pass the information about unchanged parts of the frame by means of 
> the AVVideoHint side data.
> ---
>  Changelog            |  1 +
>  doc/APIchanges       |  3 ++
>  libavcodec/libx264.c | 94 
> ++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h |  2 +-
>  4 files changed, 99 insertions(+), 1 deletion(-)
>
> diff --git a/Changelog b/Changelog
> index 3876082844..70b0fe94a3 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -25,6 +25,7 @@ version <next>:
>  - Raw VVC bitstream parser, muxer and demuxer
>  - Bitstream filter for editing metadata in VVC streams
>  - Bitstream filter for converting VVC from MP4 to Annex B
> +- support for the P_SKIP hinting to speed up libx264 encoding
>
>  version 6.0:
>  - Radiance HDR image support
> diff --git a/doc/APIchanges b/doc/APIchanges index 
> bfe04556d2..89ff88af15 100644
> --- a/doc/APIchanges
> +++ b/doc/APIchanges
> @@ -2,6 +2,9 @@ The last version increases of all libraries were on 
> 2023-02-09
>
>  API changes, most recent first:
>
> +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c
> +  Add mb_info option.

> Do we actually need the option? If the encoder's caller bothered with adding the side data, then I'd think it should always be used, otherwise why is it even there?

I agree, I just thought I had to update that file as well, if that's not the case feel free to remove it.
Best
Elias





NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
Carotti, Elias July 11, 2023, 10:01 p.m. UTC | #4
> +2023-06-21 - xxxxxxxxxx - lavc 60.23.100 - libx264.c
> +  Add mb_info option.

> Do we actually need the option? If the encoder's caller bothered with adding the side data, then I'd think it should always be used, otherwise why is it even there?

 >I agree, I just thought I had to update that file as well, if that's not the case feel free to remove it.
>Best
>Elias

On a second thought, though, I think I misinterpreted your mail and I  wrongly thought you were referring only to the APIchanges file. 

The thing is that libx264 expects a boolean value (b_mb_info) to be set to check the mb_info. 
So, yes, in principle I'd agree with you but I think we still need to tell it in advance by setting the option because the encoder needs to track additional statistics.

From x264.h:
        int          b_mb_info;            /* Use input mb_info data in x264_picture_t */
and then subsequent uses both in frame.h and analyse.c check for the flag to be set.

Further on, this flag doesn't seem to be updated by the x264_encoder_reconfig(..) method, so the only possibility I see is setting it in advance in the setup phase.
Best
Elias



NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
Stefano Sabatini Aug. 8, 2023, 8:11 a.m. UTC | #5
On date Monday 2023-07-03 15:55:40 +0000, Carotti, Elias wrote:
> On Wed, 2023-06-21 at 15:57 +0000, Carotti, Elias wrote:
> > Hi all,
> > please find the second part of the patch set.
> > Best,
> > Elias
> 
> Hi all,
> please find the second part of the patch, updating libavcodec/libx264.c
> to use the AVVideoHint side data. This should use the block_size field
> to scan the AVVideoRect array.
> 
> Best,
> Elias
> 
> 
> 
> NICE SRL, viale Monte Grappa 3/5, 20124 Milano, Italia, Registro delle Imprese di Milano Monza Brianza Lodi REA n. 2096882, Capitale Sociale: 10.329,14 EUR i.v., Cod. Fisc. e P.IVA 01133050052, Societa con Socio Unico
> 
> 

> From 5bdb624d8dbcff96493a63f02f68a3961cf72723 Mon Sep 17 00:00:00 2001
> From: Elias Carotti <eliascrt@amazon.it>
> Date: Tue, 20 Jun 2023 19:29:08 +0200
> Subject: [PATCH 2/2] lavc/libx264: add mb_info option
> 
> Pass the information about unchanged parts of the frame by means of the
> AVVideoHint side data.
> ---
>  Changelog            |  1 +
>  doc/APIchanges       |  3 ++
>  libavcodec/libx264.c | 94 ++++++++++++++++++++++++++++++++++++++++++++
>  libavcodec/version.h |  2 +-
>  4 files changed, 99 insertions(+), 1 deletion(-)

Rebased and applied, thanks.
diff mbox series

Patch

From 2afd999f38fc3a7c3f56aba1d874de00bc9575ba Mon Sep 17 00:00:00 2001
From: Elias Carotti <eliascrt _at_ amazon _dot_ it>
Date: Tue, 20 Jun 2023 19:29:08 +0200
Subject: [PATCH 2/2] lavc/libx264: add mb_info option

Pass the information about unchanged parts of the frame by means of the
AVVideoHint side data.
---
 Changelog            |  1 +
 doc/APIchanges       |  3 ++
 libavcodec/libx264.c | 91 ++++++++++++++++++++++++++++++++++++++++++++
 libavcodec/version.h |  2 +-
 4 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/Changelog b/Changelog
index 9cf3df8d6f..ec1b848b04 100644
--- a/Changelog
+++ b/Changelog
@@ -21,6 +21,7 @@  version <next>:
 - Essential Video Coding frame merge bsf
 - bwdif_cuda filter
 - Microsoft RLE video encoder
+- support for the P_SKIP hinting to speed up libx264 encoding

 version 6.0:
 - Radiance HDR image support
diff --git a/doc/APIchanges b/doc/APIchanges
index bfe04556d2..02723f5db7 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -2,6 +2,9 @@  The last version increases of all libraries were on 2023-02-09

 API changes, most recent first:

+2023-06-21 - xxxxxxxxxx - lavc 60.22.100 - libx264.c
+  Add mb_info option.
+
 2023-06-21 - xxxxxxxxxx - lavu 58.14.100 - video_hint.h
   Add AVVideoHint API.

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 5736f1efa7..c42e7ad39d 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -30,6 +30,7 @@ 
 #include "libavutil/stereo3d.h"
 #include "libavutil/time.h"
 #include "libavutil/intreadwrite.h"
+#include "libavutil/video_hint.h"
 #include "avcodec.h"
 #include "codec_internal.h"
 #include "encode.h"
@@ -48,6 +49,9 @@ 
 // from x264.h, for quant_offsets, Macroblocks are 16x16
 // blocks of pixels (with respect to the luma plane)
 #define MB_SIZE 16
+#define MB_LSIZE 4
+#define MB_FLOOR(x)      ((x) >> (MB_LSIZE))
+#define MB_CEIL(x)       MB_FLOOR((x) + (MB_SIZE - 1))

 typedef struct X264Opaque {
 #if FF_API_REORDERED_OPAQUE
@@ -123,6 +127,8 @@  typedef struct X264Context {
      * encounter a frame with ROI side data.
      */
     int roi_warned;
+
+    int mb_info;
 } X264Context;

 static void X264_log(void *p, int level, const char *fmt, va_list args)
@@ -295,6 +301,7 @@  static void free_picture(x264_picture_t *pic)
         av_free(pic->extra_sei.payloads[i].payload);
     av_freep(&pic->extra_sei.payloads);
     av_freep(&pic->prop.quant_offsets);
+    av_freep(&pic->prop.mb_info);
     pic->extra_sei.num_payloads = 0;
 }

@@ -320,6 +327,74 @@  static enum AVPixelFormat csp_to_pixfmt(int csp)
     return AV_PIX_FMT_NONE;
 }

+static void av_always_inline mbinfo_compute_changed_coords(const AVVideoRect *rect,
+                                                           int *min_x,
+                                                           int *max_x,
+                                                           int *min_y,
+                                                           int *max_y)
+{
+    *min_y = MB_FLOOR(rect->y);
+    *max_y = MB_CEIL(rect->y + rect->height);
+    *min_x = MB_FLOOR(rect->x);
+    *max_x = MB_CEIL(rect->x + rect->width);
+}
+
+static void av_always_inline mbinfo_compute_constant_coords(const AVVideoRect *rect,
+                                                            int *min_x,
+                                                            int *max_x,
+                                                            int *min_y,
+                                                            int *max_y)
+{
+    *min_y = MB_CEIL(rect->y);
+    *max_y = MB_FLOOR(rect->y + rect->height);
+    *min_x = MB_CEIL(rect->x);
+    *max_x = MB_FLOOR(rect->x + rect->width);
+}
+
+static int setup_mb_info(AVCodecContext *ctx, x264_picture_t *pic,
+                         const AVFrame *frame,
+                         const AVVideoHint *info)
+{
+    int mb_width = (frame->width + MB_SIZE - 1) / MB_SIZE;
+    int mb_height = (frame->height + MB_SIZE - 1) / MB_SIZE;
+
+    const AVVideoRect *mbinfo_rects;
+    int nb_rects;
+    uint8_t *mbinfo;
+
+    mbinfo_rects = (const AVVideoRect *)av_video_hint_rects(info);
+    nb_rects = info->nb_rects;
+
+    mbinfo = av_calloc(mb_width * mb_height, sizeof(*mbinfo));
+    if (!mbinfo)
+        return AVERROR(ENOMEM);
+
+#define COMPUTE_MBINFO(mbinfo_filler_, mbinfo_marker_, compute_coords_fn_) \
+    memset(mbinfo, mbinfo_filler_, sizeof(*mbinfo) * mb_width * mb_height); \
+                                                                        \
+    for (int i = 0; i < nb_rects; i++) {                                \
+        int min_x, max_x, min_y, max_y;                                 \
+                                                                        \
+        compute_coords_fn_(mbinfo_rects, &min_x, &max_x, &min_y, &max_y); \
+        for (int mb_y = min_y; mb_y < max_y; ++mb_y) {                  \
+            memset(mbinfo + mb_y * mb_width + min_x, mbinfo_marker_, max_x - min_x); \
+        }                                                               \
+                                                                        \
+        mbinfo_rects++;                                                 \
+    }                                                                   \
+
+    if (info->type == AV_VIDEO_HINT_CHANGED) {
+        COMPUTE_MBINFO(X264_MBINFO_CONSTANT, 0, mbinfo_compute_changed_coords);
+    } else /* if (info->type == AV_VIDEO_HINT_CHANGED) */ {
+        COMPUTE_MBINFO(0, X264_MBINFO_CONSTANT, mbinfo_compute_constant_coords);
+    }
+
+    pic->prop.mb_info = mbinfo;
+    pic->prop.mb_info_free = av_free;
+
+    return 0;
+}
+
 static int setup_roi(AVCodecContext *ctx, x264_picture_t *pic, int bit_depth,
                      const AVFrame *frame, const uint8_t *data, size_t size)
 {
@@ -404,6 +479,7 @@  static int setup_frame(AVCodecContext *ctx, const AVFrame *frame,
     int64_t wallclock = 0;
     int bit_depth, ret;
     AVFrameSideData *sd;
+    AVFrameSideData *mbinfo_sd;

     *ppic = NULL;
     if (!frame)
@@ -499,6 +575,17 @@  FF_ENABLE_DEPRECATION_WARNINGS
             goto fail;
     }

+    mbinfo_sd = av_frame_get_side_data(frame, AV_FRAME_DATA_VIDEO_HINT);
+    if (mbinfo_sd) {
+        int ret = setup_mb_info(ctx, pic, frame, (const AVVideoHint *)mbinfo_sd->data);
+        if (ret < 0) {
+            /* No need to fail here, this is not fatal. We just proceed with no
+             * mb_info and log a message */
+
+            av_log(ctx, AV_LOG_WARNING, "setup_mb_info failed with error: %s\n", av_err2str(ret));
+        }
+    }
+
     if (x4->udu_sei) {
         for (int j = 0; j < frame->nb_side_data; j++) {
             AVFrameSideData *side_data = frame->side_data[j];
@@ -1102,6 +1189,9 @@  FF_ENABLE_DEPRECATION_WARNINGS
         }
     }

+    x4->params.analyse.b_mb_info = x4->mb_info;
+    x4->params.analyse.b_fast_pskip = 1;
+
     // update AVCodecContext with x264 parameters
     avctx->has_b_frames = x4->params.i_bframe ?
         x4->params.i_bframe_pyramid ? 2 : 1 : 0;
@@ -1311,6 +1401,7 @@  static const AVOption options[] = {
     { "noise_reduction", "Noise reduction",                               OFFSET(noise_reduction), AV_OPT_TYPE_INT, { .i64 = -1 }, INT_MIN, INT_MAX, VE },
     { "udu_sei",      "Use user data unregistered SEI if available",      OFFSET(udu_sei),  AV_OPT_TYPE_BOOL,   { .i64 = 0 }, 0, 1, VE },
     { "x264-params",  "Override the x264 configuration using a :-separated list of key=value parameters", OFFSET(x264_params), AV_OPT_TYPE_DICT, { 0 }, 0, 0, VE },
+    { "mb_info",      "Set mb_info data through AVSideData, only useful when used from the API", OFFSET(mb_info), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, VE },
     { NULL },
 };

diff --git a/libavcodec/version.h b/libavcodec/version.h
index da6f3a84ac..9411511e04 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@ 

 #include "version_major.h"

-#define LIBAVCODEC_VERSION_MINOR  21
+#define LIBAVCODEC_VERSION_MINOR  22
 #define LIBAVCODEC_VERSION_MICRO 100

 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
--
2.34.1