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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
