From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id B15CB4517B for ; Sun, 11 Jun 2023 17:15:24 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EC91068C35A; Sun, 11 Jun 2023 20:15:20 +0300 (EEST) Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 26D3A68BFA0 for ; Sun, 11 Jun 2023 20:15:14 +0300 (EEST) Received: by mail-wr1-f51.google.com with SMTP id ffacd0b85a97d-30af56f5f52so2276074f8f.1 for ; Sun, 11 Jun 2023 10:15:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1686503713; x=1689095713; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=VWuh0zI/WWWmPUFs83XEG/No61+i85wFX7+nJiY7Kbs=; b=LJpXNNIrZbjwrkJZDMqQGDR1rsm4xtxdMeo3BOYV+suhvAtExB/5//k4iJcgMNW+p9 Up/SSEZpGRDkWYwsqOSZEskP2CYVz8q6kBhAJSYQITd5I8UNKSDsz3GdQbyaJiHTMt83 EF3SJ3NWl7IunfhPiWTn4J0DWRUuk+JDOGsF5n3xnDJXvUXZ0JLglJrrXgKYPdGEJNa2 wcYzHhfHBJRa5so1vPm7/2UA75iLP8c4SabiBWfkTlGUuWAKKwRN4WteA923WufJ63/o uTJfZF+7CAN25YYokhLCnSAe7hVcox26gnRpm+geLKxsrxtiQ1tfABkLNdysTDUIKZzZ OuvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686503713; x=1689095713; h=user-agent:in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=VWuh0zI/WWWmPUFs83XEG/No61+i85wFX7+nJiY7Kbs=; b=DOiYemVoCB3AuFGKZ1hfcAZRlMB7auWwgS54GQ7gc0vTaZm5J50M4l7pYeF2AJOM7a 3S5xi+EhTgC7eZ4IO3M7QWgzNWlk6JMPRo0NzFSFLTKXovbkzOgUmvjx/3E0DkelAdl/ QCZZkx5ihrOAXqBhhW7h9LqDoDSEZs7DBzliAybHhXP/SiexgphhlIueuaqJG4JmkDgw kjsQVn2rWcnY8romzzh3sLZ+9Oi1G7jwkpcERRxbsmHjEQhuxxHlJ4nVa7Zly0GD5+xI gWHtxqvMMhmuniWy1C1t9yWUk6V8Mm2r+pTqXiKNa/HTLR+a734IBs32lvARju8avz02 2TyA== X-Gm-Message-State: AC+VfDynTfaPD7oqf/SXU51GQEILIvupiGj0Q8jtpNlMZsfjitgjOkMV O3ja0w8kEtfnGMYtxzqbNXu77T1COkE= X-Google-Smtp-Source: ACHHUZ62dClmZuVaOoIYEQQxj70z2KthizPkPOrCtXTxZpiwX1nfmXigB7WNguko0f5umjGMinEIDg== X-Received: by 2002:a05:6000:1951:b0:30a:dee7:e48e with SMTP id e17-20020a056000195100b0030adee7e48emr2683392wry.8.1686503712344; Sun, 11 Jun 2023 10:15:12 -0700 (PDT) Received: from mariano ([151.56.109.141]) by smtp.gmail.com with ESMTPSA id s1-20020a5d4ec1000000b002c70ce264bfsm10126947wrv.76.2023.06.11.10.15.11 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 11 Jun 2023 10:15:11 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id A745CBFB73; Sun, 11 Jun 2023 19:15:10 +0200 (CEST) Date: Sun, 11 Jun 2023 19:15:10 +0200 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: <20230611171510.GA53039@mariano> Mail-Followup-To: FFmpeg development discussions and patches References: <8eb9290f3c9cdae75b38c45a5d49ee624951a849.camel@amazon.it> <20230521231741.GB14958@mariano> <20230604152912.GC4274@mariano> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.13.2 (2019-12-18) Subject: Re: [FFmpeg-devel] [PATCH] Optimization: support for libx264's mb_info X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: 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 > 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 > + * > + * 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 > + > +#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@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".