[FFmpeg-devel] [PATCH] Optimization: support for libx264's mb_info
Carotti, Elias
eliascrt at amazon.it
Mon Jun 12 20:28:10 EEST 2023
Hi Stefano,
Here is the revised patch according to your suggestions. This should
allow for efficient inlining of the methods computing the map of
skipped macrobloks.
Best,
Elias
On Sun, 2023-06-11 at 19:15 +0200, Stefano Sabatini 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 date Monday 2023-06-05 15:32:35 +0000, Carotti, Elias wrote:
> > Hi,
> > please find attached the patch which I updated according to your
> > suggestions.
> > Best,
> > Elias
> [...]
>
> > From 8288d2bd36ffed29140d46c42b6f5515a9058836 Mon Sep 17 00:00:00
> > 2001
> > From: Elias Carotti <eliascr _at_ amazon _dot_ it>
> > Date: Wed, 19 Apr 2023 11:49:39 +0200
> > Subject: [PATCH] Add support for libx264's MB_INFO
> >
> > libx264's x264_image_properties_t, which is passed to the encoding
> > function,
> > contains a field to pass down information on the portions of the
> > frame which
> > changed with respect to the previous one (used for prediction) to
> > mark
> > unchanged macroblocks P_SKIP.
> > ---
> > libavcodec/libx264.c | 94
> > +++++++++++++++++++++++++++++++++++++
> > libavutil/Makefile | 4 ++
> > libavutil/frame.h | 10 ++++
> > libavutil/video_hint_info.c | 89
> > +++++++++++++++++++++++++++++++++++
> > libavutil/video_hint_info.h | 87
> > ++++++++++++++++++++++++++++++++++
> > 5 files changed, 284 insertions(+)
> > create mode 100644 libavutil/video_hint_info.c
> > create mode 100644 libavutil/video_hint_info.h
> >
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index 5736f1efa7..2cf7755eec 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_info.h"
> > #include "avcodec.h"
> > #include "codec_internal.h"
> > #include "encode.h"
> > @@ -48,6 +49,13 @@
> > // 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 void (*AVMBInfoComputeCoords)(const AVVideoRect *rect,
> > + int *min_x, int *max_x,
> > + int *min_y, int *max_y);
> >
> > typedef struct X264Opaque {
> > #if FF_API_REORDERED_OPAQUE
> > @@ -123,6 +131,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 +305,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 +331,73 @@ static enum AVPixelFormat csp_to_pixfmt(int
> > csp)
> > return AV_PIX_FMT_NONE;
> > }
> >
> > +static void 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 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;
> > + int mbinfo_filler;
> > + int mbinfo_marker;
> > + AVMBInfoComputeCoords compute_coords_fn;
> > +
> > + 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);
> > +
>
> > + mbinfo_filler = (info->type == AV_VIDEO_HINT_CHANGED) ?
> > X264_MBINFO_CONSTANT : 0;
> > + mbinfo_marker = (info->type == AV_VIDEO_HINT_CHANGED) ? 0
> > : X264_MBINFO_CONSTANT;
> > + compute_coords_fn = (info->type == AV_VIDEO_HINT_CHANGED) ?
> > mbinfo_compute_changed_coords : mbinfo_compute_constant_coords;
> > +
> > + 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++;
> > + }
>
> maybe
>
> #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);
> }
>
> this adds to spatial complexity but enables the use of inlined
> functions to avoid the function call in the loop
>
> > + 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 +482,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 +578,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, "mb_info setup
> > failure\n");
>
> nit to provide more context: "setup_mb_info failed with error: %s\n",
> av_strerror(ret)
>
> > + }
> > + }
> > +
> > if (x4->udu_sei) {
> > for (int j = 0; j < frame->nb_side_data; j++) {
> > AVFrameSideData *side_data = frame->side_data[j];
> > @@ -1102,6 +1192,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 +1404,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 },
> > };
> [...]
> > enum AVActiveFormatDescription {
> > diff --git a/libavutil/video_hint_info.c
> > b/libavutil/video_hint_info.c
> > new file mode 100644
> > index 0000000000..c920bd6232
> > --- /dev/null
> > +++ b/libavutil/video_hint_info.c
> > @@ -0,0 +1,89 @@
> > +/*
> > + * Copyright 2023 Elias Carotti <eliascrt at amazon dot it>
> > + *
> > + * 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 "video_hint_info.h"
> > +
> > +AVVideoHint *av_video_hint_alloc(AVVideoRect *rects,
> > + size_t nb_rects,
> > + AVVideoHintType type,
> > + size_t* out_size)
> > +{
> > + struct TestStruct {
> > + AVVideoHint hint;
> > + AVVideoRect rect;
> > + };
>
> > + const size_t blocks_offset = offsetof(struct TestStruct,
> > rect);
> > + size_t size = blocks_offset;
> > + AVVideoHint *hint;
> > +
> > + *out_size = 0;
> > + if (nb_rects > (SIZE_MAX - size) / sizeof(AVVideoRect))
> > + return NULL;
> > + size += sizeof(AVVideoRect) * nb_rects;
> > +
> > + hint = av_mallocz(size);
> > + if (!hint)
> > + return NULL;
> > +
> > + hint->type = type;
> > + hint->nb_rects = nb_rects;
> > + hint->blocks_offset = blocks_offset;
> > +
> > + /* Just copies the rects over the newly allocated buffer */
> > + memcpy((uint8_t *)hint + blocks_offset, rects,
> > sizeof(AVVideoRect) * nb_rects);
> > +
> > + *out_size = size;
> > +
> > + return hint;
> > +}
> > +
> > +AVVideoHint *av_video_hint_create_side_data(AVFrame *frame,
> > + AVVideoRect *rects,
> > + size_t num_rects,
> > + AVVideoHintType type)
> > +{
> > + AVVideoHint *hint;
> > + AVBufferRef *buf;
> > + size_t size = 0;
> > +
> > + hint = av_video_hint_alloc(rects, num_rects, type, &size);
> > + if (!hint)
> > + return NULL;
> > +
> > + buf = av_buffer_create((uint8_t *)hint, size, NULL, NULL, 0);
> > + if (!buf) {
> > + av_freep(&hint);
> > + return NULL;
> > + }
> > +
> > + if (!av_frame_new_side_data_from_buf(frame,
> > AV_FRAME_DATA_VIDEO_HINT, buf)) {
> > + av_buffer_unref(&buf);
> > + return NULL;
> > + }
> > +
> > + return hint;
> > +}
> > +
> > diff --git a/libavutil/video_hint_info.h
> > b/libavutil/video_hint_info.h
> > new file mode 100644
> > index 0000000000..2844398d18
> > --- /dev/null
>
> > +++ b/libavutil/video_hint_info.h
>
> strip the _info part since to have simple mapping between data
> structure and file names
>
> [...]
>
> Looks good to me otherwise, maybe Michael/Anton or someone else want
> to have a look?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-support-for-libx264-s-MB_INFO.patch
Type: text/x-patch
Size: 15963 bytes
Desc: 0001-Add-support-for-libx264-s-MB_INFO.patch
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20230612/566a49f5/attachment.bin>
More information about the ffmpeg-devel
mailing list