Re: [FFmpeg-devel] [PATCH] Added support for MB_INFO

["Carotti, Elias" <eliascrt@amazon.it>]

[-- Attachment #1: Type: text/plain, Size: 3993 bytes --]

Sorry to revive an old thread, but I updated the patch for ffmpeg 6 and
this new patch should address the comments.
Still this is a libx264-only patch, and provides a means to specify
that only portions of the frame have changed from the previous one
while the others should be P_SKIP-ped.

More specifically, now the data is fully contained in the side data and
the actual map of macroblocks which changed from the previous frame is
created from the side information at encoding time, much like what is
done for the region of interest.

best,
Elias


On Fri, 2022-06-17 at 17:10 +0200, Timo Rothenpieler wrote:
> CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender
> and know the content is safe.
> 
> 
> 
> On 17.06.2022 12:59, Carotti, Elias wrote:
> > > 
> > > Yes, exactly. It relies on x264 to free it.
> > 
> > Not really. It can rely on x264 if you explicitly want that
> > behavior.
> > If you do not set a deallocator, it remains the caller
> > responsibility.
> > 
> > > What happens if x264 is not involved, but some other encoder,
> > > which
> > > does
> > > not check for that kind of side data?
> > > 
> > > How does the caller know that the data was consumed, and the
> > > ownership
> > > passed on?
> > 
> > Again, you don't have to pass the ownership, and in fact in my use
> > case
> > I do not pass it since I actually recycle and update the same
> > buffer
> > for subsequent frames. If you do intentionally pass the ownership
> > you
> > need to be aware of what you are doing. As I said, I see a leak
> > only in
> > case of a programming error.
> > Maybe we could explicitly state it in the comment section in
> > mb_info.h:
> > if you set the deallocator you're relinquishing ownership of the
> > buffer.
> 
> I'm not sure if that's something you can sensibly do with side data?
> What if it gets buffered, copied, and so on?
> 
> > Plus, there's one more flag (b_mb_info_update) in libx264 which
> > allows
> > to pass back information to the caller about which macroblocks
> > among
> > the flagged ones were actually encoded as P_SKIP. I did not add
> > support
> > for that though but in the future somebody may want to.
> 
> Yes, it's very x264 specific.
> But the side data is generic. If for some reason x264 does not
> process a
> frame, or any other encoder ends up getting used, the data will leak
> if
> it relied on x264 to free it.
> 
> > In principle I could've put the buffer into the side information 
> > and
> > not just pass a pointer to it but I thought that it would require
> > adding more semantics which would imply a stronger dependency on
> > libx264.
> > Right now, mb_info is a vector of uint8_t flags and the only
> > possible
> > value - to date - is X264_MBINFO_CONSTANT. What if, say, one day
> > libx264 decides the buffer has to be a vector of uint16_t or still
> > uint8_t but the flags are packed? On the other hand, this, AFAIK,
> > is
> > only supported by libx264. Other codecs may want to choose a
> > different
> > semantic for a similar field and the only possibility to make it
> > generic is letting the caller handling the low level details.
> 
> I'm not aware of any other side data with a similar semantic. And I'm
> really not sure if it's sane or even valid to do it like that.
> Can't the data be entirely contained within the side data?
> _______________________________________________
> 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".




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



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-support-for-x264-s-MB_INFO.patch --]
[-- Type: text/x-patch; name="0001-Add-support-for-x264-s-MB_INFO.patch", Size: 11120 bytes --]

From 6c9024de2d0cdb03706ef8894a36f954185cefbb Mon Sep 17 00:00:00 2001
From: Elias Carotti <eliascrt@amazon.it>
Date: Wed, 19 Apr 2023 11:49:39 +0200
Subject: [PATCH] Add support for x264's MB_INFO

---
 libavcodec/libx264.c | 61 ++++++++++++++++++++++++++++++++++++++++++++
 libavutil/Makefile   |  4 +++
 libavutil/frame.h    | 10 ++++++++
 libavutil/mb_info.c  | 48 ++++++++++++++++++++++++++++++++++
 libavutil/mb_info.h  | 46 +++++++++++++++++++++++++++++++++
 5 files changed, 169 insertions(+)
 create mode 100644 libavutil/mb_info.c
 create mode 100644 libavutil/mb_info.h

diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index cfdd422236..a98d702c92 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/mb_info.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)
@@ -320,6 +326,45 @@ static enum AVPixelFormat csp_to_pixfmt(int csp)
     return AV_PIX_FMT_NONE;
 }
 
+static int setup_mb_info(AVCodecContext *ctx, x264_picture_t *pic,
+                         const AVFrame *frame, const uint8_t *data,
+                         size_t size)
+{
+    int mb_width = (frame->width + MB_SIZE - 1) / MB_SIZE;
+    int mb_height = (frame->height + MB_SIZE - 1) / MB_SIZE;
+    const AVMBInfoRect *mbinfo_rects;
+    size_t mbinfo_count;
+    uint8_t *mbinfo;
+
+    mbinfo_rects = (const AVMBInfoRect *)data;
+    mbinfo_count = size / sizeof(AVMBInfoRect);
+
+    mbinfo = av_calloc(mb_width * mb_height, sizeof(*mbinfo));
+    if (!mbinfo)
+        return AVERROR(ENOMEM);
+
+    /* Sets the default as constant, i.e. P_SKIP-able, then selectively resets the flag */
+    memset(mbinfo, X264_MBINFO_CONSTANT, sizeof(*mbinfo) * mb_width * mb_height);
+
+    for (int i = 0; i < mbinfo_count; i++) {
+        int min_y = MB_FLOOR(mbinfo_rects->y);
+        int max_y = MB_CEIL(mbinfo_rects->y + mbinfo_rects->height);
+        int min_x = MB_FLOOR(mbinfo_rects->x);
+        int max_x = MB_CEIL(mbinfo_rects->x + mbinfo_rects->width);
+
+        for (int mb_y = min_y; mb_y < max_y; ++mb_y) {
+            memset(mbinfo + mb_y * mb_width + min_x, 0, max_x - min_x);
+        }
+
+        mbinfo_rects++;
+    }
+
+    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 +449,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 +545,17 @@ FF_ENABLE_DEPRECATION_WARNINGS
             goto fail;
     }
 
+    mbinfo_sd = av_frame_get_side_data(frame, AV_FRAME_DATA_MB_INFO);
+    if (mbinfo_sd) {
+        int ret = setup_mb_info(ctx, pic, frame, mbinfo_sd->data, mbinfo_sd->size);
+        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, "mb_info setup failure\n");
+        }
+    }
+
     if (x4->udu_sei) {
         for (int j = 0; j < frame->nb_side_data; j++) {
             AVFrameSideData *side_data = frame->side_data[j];
@@ -1096,6 +1153,9 @@ static av_cold int X264_init(AVCodecContext *avctx)
         }
     }
 
+    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;
@@ -1305,6 +1365,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/libavutil/Makefile b/libavutil/Makefile
index dc9012f9a8..e99f448213 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -91,6 +91,7 @@ HEADERS = adler32.h                                                     \
           tea.h                                                         \
           tx.h                                                          \
           film_grain_params.h                                           \
+          mb_info.h                                                     \
 
 ARCH_HEADERS = bswap.h                                                  \
                intmath.h                                                \
@@ -196,6 +197,7 @@ OBJS-$(CONFIG_VAAPI)                    += hwcontext_vaapi.o
 OBJS-$(CONFIG_VIDEOTOOLBOX)             += hwcontext_videotoolbox.o
 OBJS-$(CONFIG_VDPAU)                    += hwcontext_vdpau.o
 OBJS-$(CONFIG_VULKAN)                   += hwcontext_vulkan.o
+OBJS-$(CONFIG_LIBX264)                  += mb_info.o
 
 OBJS-$(!CONFIG_VULKAN)                  += hwcontext_stub.o
 
@@ -219,6 +221,8 @@ SKIPHEADERS-$(CONFIG_VULKAN)           += hwcontext_vulkan.h vulkan.h   \
                                           vulkan_functions.h            \
                                           vulkan_loader.h
 
+SKIPHEADERS-$(CONFIG_LIBX264)  	       += mb_info.h
+
 TESTPROGS = adler32                                                     \
             aes                                                         \
             aes_ctr                                                     \
diff --git a/libavutil/frame.h b/libavutil/frame.h
index f85d630c5c..9c0fdcf25d 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -214,6 +214,16 @@ enum AVFrameSideDataType {
      * Ambient viewing environment metadata, as defined by H.274.
      */
     AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT,
+
+    /**
+     * Provide macro block encoder-specific hinting information for the encoder
+     * processing.  It can be used to pass information about which macroblock
+     * can be skipped because it hasn't changed from the corresponding one in
+     * the previous frame. This is useful for applications which know in
+     * advance this information to speed up real-time encoding.  Currently only
+     * used by libx264.
+     */
+    AV_FRAME_DATA_MB_INFO,
 };
 
 enum AVActiveFormatDescription {
diff --git a/libavutil/mb_info.c b/libavutil/mb_info.c
new file mode 100644
index 0000000000..15e8df1940
--- /dev/null
+++ b/libavutil/mb_info.c
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2023 Elias Carotti <eliascrt at amazon dot com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <string.h>
+
+#include "avstring.h"
+#include "frame.h"
+#include "macros.h"
+#include "mem.h"
+#include "mb_info.h"
+
+
+AVMBInfoRect *av_mb_info_create_side_data(AVFrame *frame,
+                                          AVMBInfoRect *rects,
+                                          size_t num_rects)
+{
+    AVFrameSideData *side_data;
+    AVMBInfoRect *par;
+
+    side_data = av_frame_new_side_data(frame,
+                                       AV_FRAME_DATA_MB_INFO,
+                                       num_rects * sizeof(AVMBInfoRect));
+
+    par  = (AVMBInfoRect *)side_data->data;
+
+    /* Just copies the rects over the newly allocated buffer */
+    memcpy(par, rects, sizeof(AVMBInfoRect) * num_rects);
+
+    return par;
+}
+
diff --git a/libavutil/mb_info.h b/libavutil/mb_info.h
new file mode 100644
index 0000000000..918cf167aa
--- /dev/null
+++ b/libavutil/mb_info.h
@@ -0,0 +1,46 @@
+/**
+ * Copyright 2023 Elias Carotti <eliascrt at amazon dot com>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#ifndef AVUTIL_MB_INFO_H
+#define AVUTIL_MB_INFO_H
+
+#include <stddef.h>
+#include <stdint.h>
+#include "libavutil/avassert.h"
+#include "libavutil/frame.h"
+
+typedef struct _AVMBInfoRect {
+    uint32_t x, y;
+    uint32_t width, height;
+} AVMBInfoRect;
+
+/**
+ * Allocate memory for a vector of AVMBInfoRect in the given AVFrame
+ * {@code frame} as AVFrameSideData of type AV_FRAME_DATA_MB_INFO.
+ * The side data contains a list of rectangles for the portions of the frame
+ * which changed from the last encoded one. The rest will be hinted to be
+ * P_SKIP-ped.  Portions of the rects which are not on macroblock boundaries
+ * are not handled as P_SKIPS.
+ */
+AVMBInfoRect *av_mb_info_create_side_data(AVFrame *frame,
+                                          AVMBInfoRect *rects,
+                                          size_t num_rects);
+
+#endif /* AVUTIL_MB_INFO_H */
-- 
2.34.1


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".