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 9BCC446455 for ; Mon, 17 Jul 2023 22:19:53 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 6DA1468C649; Tue, 18 Jul 2023 01:19:50 +0300 (EEST) Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D8C4568C38E for ; Tue, 18 Jul 2023 01:19:43 +0300 (EEST) Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-3fbc12181b6so52866465e9.2 for ; Mon, 17 Jul 2023 15:19:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689632382; x=1692224382; 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=ctWiCwT2msBL6yo39ccW60wiPyC3ZPYuGA6d1uZz0GI=; b=RstJQKFmOFYjSHr3pOsWypStwAqcdTmlfg+XE8Xw1aUKc3x0D6qFR/iu6RAURwKjCr nYokIi1QBTzt0JB3Q6yVuVTlEY21dYZ04t9ThsZ+7Guw8eGMMjpdrd09NmrUrlgWGjQu ZsoAAcCq0VUHxqoCZ7WjgC41D1pkO/gdHGdZfEGYiYOpyOGUKo/qdaLO2trnEqq2U4NU cz9SRMNgPxp5s8Dq7N6RgR26y+Q0FR0oUkENWSO90C+C7uhrT6pGZmlSNd9MDv4JcD9z CRNEzTi33flAMSWRvOhoI/tzVwXVDQprvxHeCIqFLD8JaU+Vs3FkJI6rAUsSMsGdvOwG 6/Cw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689632382; x=1692224382; 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=ctWiCwT2msBL6yo39ccW60wiPyC3ZPYuGA6d1uZz0GI=; b=WX8uUu4RMFJAe7j/BjGkKjZfUw8BWcuGX54Q4Wc4Mueba0yHkXCcMix1HpMO62pBsz VR1iE+RccwxDN6/XZWOipLolu70hK6nJfqlYwcYmFhqKTkjFcVsP7i5SBJ+M0iB/MLa0 bTtNDnJ/Wcq8Nv2Z955DJehOwA6V4t2Vst0hhB5dU61zvUcvlsJzQ16Q0n/x15L1stkF ap2avpJyJx1RKQzhODyHQiuShdqXuEzITmK2dOCJ6U6D01srVEFwZgCZS5w6FBNhPBDf jaWGpahOqKAj5AHUpJm0chRXZq4UOEzAN0zdLHA0yqkSPP/45akd2Riz+2m+tzQD0g1W YbnQ== X-Gm-Message-State: ABy/qLaBEi9RXNWPbOgpNQ057fRJZQ9eyiCenf8iqdZgIORARDo9wRJP 1DO1i2qZljQ6CCeinglbiPX0MojVDSk= X-Google-Smtp-Source: APBJJlFfubNETXfCsH7EZglYR2zOw3FVQpqfCeHNQf/IeYXKOT9rcA/rs4+DUOY2VQlljXEJOUT2Ww== X-Received: by 2002:a05:600c:216:b0:3fa:9850:8b1d with SMTP id 22-20020a05600c021600b003fa98508b1dmr447420wmi.30.1689632381720; Mon, 17 Jul 2023 15:19:41 -0700 (PDT) Received: from mariano ([151.56.88.193]) by smtp.gmail.com with ESMTPSA id b17-20020a5d40d1000000b003140f47224csm567951wrq.15.2023.07.17.15.19.40 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Jul 2023 15:19:40 -0700 (PDT) Received: by mariano (Postfix, from userid 1000) id 26934BFB73; Tue, 18 Jul 2023 00:19:40 +0200 (CEST) Date: Tue, 18 Jul 2023 00:19:40 +0200 From: Stefano Sabatini To: FFmpeg development discussions and patches Message-ID: <20230717221940.GC10400@mariano> Mail-Followup-To: FFmpeg development discussions and patches References: <2fe8e0027d33c25e2de01e57ea1dd7b0cef300bf.camel@amazon.it> <168795695865.21886.2879227621245771333@lain.khirnov.net> <168820043273.21886.17648463695363628461@lain.khirnov.net> <20230709110529.29490-1-anton@khirnov.net> <2ef05e377ecdcb086d2ec00a0c8d810ff962ca77.camel@amazon.it> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <2ef05e377ecdcb086d2ec00a0c8d810ff962ca77.camel@amazon.it> User-Agent: Mutt/1.13.2 (2019-12-18) Subject: Re: [FFmpeg-devel] [PATCH] lavu: add AVVideoHint API 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-07-10 12:51:25 +0000, Carotti, Elias wrote: > On Mon, 2023-07-10 at 08:13 +0000, Carotti, Elias wrote: > > > > > AVVideoHint is a bad name for something like this. > > Could you borrow some wording from graphics and call it > > AVVideoDamagedHint or maybe AVVideoChangedAreaHint or a combination > > of both? > > I'd prefer the former, damage is standard language in graphics > > circles about what has changed since the last frame. > > > > Hi, > > I have no strong objections on this. Personally I also like the > > AVVideoDamagedHint name best, my only concern is that it is strictly > > related to the current use/implementation > > (it's true right now that's the only kind of hint) while it may turn > > out to be a bad naming decision should other forms of hinting for the > > encoder be added in the future. > > That said, I am fine with the change too. AVVideoDamagedHint would be too specific (what if we want to add "hinting" for different things?). > > Elias > > > > I added a type to the AVVideoRect struct. This should solve the naming > issue above while preserving the possibility to extend this to > different hinting types. > These are the only changes to Anton's version. > 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 8ef4f97410a6b78df048b71d9921a763da6255b3 Mon Sep 17 00:00:00 2001 > From: Elias Carotti > Date: Mon, 10 Jul 2023 14:34:53 +0200 > Subject: [PATCH] Add side data type to provide hint to the video encoders > about unchanged portions of each frame. > > Signed-off-by: Anton Khirnov > --- > doc/APIchanges | 3 ++ > libavutil/Makefile | 2 + > libavutil/frame.h | 10 ++++ > libavutil/version.h | 2 +- > libavutil/video_hint.c | 82 +++++++++++++++++++++++++++++ > libavutil/video_hint.h | 117 +++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 215 insertions(+), 1 deletion(-) > create mode 100644 libavutil/video_hint.c > create mode 100644 libavutil/video_hint.h > > diff --git a/doc/APIchanges b/doc/APIchanges > index 27f835cfce..0cda51fdee 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-07-xx - xxxxxxxxxx - lavu 58.15.100 - video_hint.h > + Add AVVideoHint API. > + > 2023-07-05 - xxxxxxxxxx - lavu 58.14.100 - random_seed.h > Add av_random_bytes() > > diff --git a/libavutil/Makefile b/libavutil/Makefile > index bd9c6f9e32..7828c94dc5 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -91,6 +91,7 @@ HEADERS = adler32.h \ > tea.h \ > tx.h \ > film_grain_params.h \ > + video_hint.h > > ARCH_HEADERS = bswap.h \ > intmath.h \ > @@ -181,6 +182,7 @@ OBJS = adler32.o \ > uuid.o \ > version.o \ > video_enc_params.o \ > + video_hint.o \ > film_grain_params.o \ > > > diff --git a/libavutil/frame.h b/libavutil/frame.h > index a491315f25..c0c1b23db7 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 encoder-specific hinting information about changed/unchanged > + * portions of a frame. It can be used to pass information about which > + * macroblocks can be skipped because they didn't change from the > + * corresponding ones in the previous frame. This could be useful for > + * applications which know this information in advance to speed up > + * encoding. > + */ > + AV_FRAME_DATA_VIDEO_HINT, > }; > > enum AVActiveFormatDescription { > diff --git a/libavutil/version.h b/libavutil/version.h > index 24af520e08..9e798b0e3f 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -79,7 +79,7 @@ > */ > > #define LIBAVUTIL_VERSION_MAJOR 58 > -#define LIBAVUTIL_VERSION_MINOR 14 > +#define LIBAVUTIL_VERSION_MINOR 15 > #define LIBAVUTIL_VERSION_MICRO 100 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > diff --git a/libavutil/video_hint.c b/libavutil/video_hint.c > new file mode 100644 > index 0000000000..5730ed6cfb > --- /dev/null > +++ b/libavutil/video_hint.c > @@ -0,0 +1,82 @@ > +/* > + * 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.h" > + > +AVVideoHint *av_video_hint_alloc(size_t nb_rects, > + size_t *out_size) > +{ > + struct TestStruct { > + AVVideoHint hint; > + AVVideoRect rect; > + }; > + const size_t rect_offset = offsetof(struct TestStruct, rect); > + size_t size = rect_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->nb_rects = nb_rects; > + hint->rect_offset = rect_offset; > + hint->rect_size = sizeof(AVVideoRect); > + > + *out_size = size; > + > + return hint; > +} > + > +AVVideoHint *av_video_hint_create_side_data(AVFrame *frame, > + size_t nb_rects) > +{ > + AVVideoHint *hint; > + AVBufferRef *buf; > + size_t size = 0; > + > + hint = av_video_hint_alloc(nb_rects, &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.h b/libavutil/video_hint.h > new file mode 100644 > index 0000000000..14bfe20aa6 > --- /dev/null > +++ b/libavutil/video_hint.h > @@ -0,0 +1,117 @@ > +/** > + * 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 > + */ > + > +#ifndef AVUTIL_VIDEO_HINT_H > +#define AVUTIL_VIDEO_HINT_H > + > +#include > +#include > +#include "libavutil/avassert.h" > +#include "libavutil/frame.h" > + > +/** > + * The type of the hinting information provided by an AVVideoRect. > + * Currently only damage information, i.e., changed/unchanged portions of the frame. > + **/ > +typedef enum AVVideoRectType { > + AV_VIDEO_RECT_DAMAGE, AV_VIDEO_RECT_TYPE_DAMAGE for consistency > +} AVVideoRectType; > + > +typedef struct AVVideoRect { > + uint32_t x, y; > + uint32_t width, height; > + AVVideoRectType type; > +} AVVideoRect; > + > +typedef enum AVVideoHintType { > + /* rectangled delimit the constant areas (unchanged), default is changed */ rectangles? > + AV_VIDEO_HINT_CONSTANT, AV_VIDEO_HINT_TYPE_... > + > + /* rectangled delimit the constant areas (changed), default is not changed */ rectangles? > + AV_VIDEO_HINT_CHANGED, > +} AVVideoHintType; > > +typedef struct AVVideoHint { > + /** > + * Number of AVVideoRect present. > + * > + * May be 0, in which case no per-rectangle information is present. In this > + * case the values of rect_offset / rect_size are unspecified and should > + * not be accessed. > + */ > + size_t nb_rects; > + > + /** > + * Offset in bytes from the beginning of this structure at which the array > + * of AVVideoRect starts. > + */ > + size_t rect_offset; > + > + /** > + * Size in bytes of AVVideoRect. > + */ > + size_t rect_size; > + > + AVVideoHintType type; sorry to nitpick, but if we have this information in the single rect should not we use that instead? Basically I see two options: 1. generic VideoHint, storing single hint type and rects for example we might have type=changed, and the rects storing only the changed rects If we want to extend this then we need to add a new type. The problem might be if we want to store more than one VideoHint in the side data (it it possible to store more than one AVVideoHint, each one with a different type, as side data?). Suppose for example we want to add a new hint type, e.g. ROI, then we can add a new AVHideoHint with type=roi - but I don't know if we can have several frame hint side data (each one with different individual types). 2. generic VideoHint, storing rects each one containing the hint type for example we might have a list of rects, each one with type=changed|constant|roi. This would allow one to store different kind of hints in the same AVVideoHint structure, but at the same time would enable inconsistent data (for example we might have changed and unchanged rects in the same list of rects) In each case, storing the type in the generic AVVideHint and in each rect might lead to inconsistent states (e.g. you might have generic type=damage, and have type=roi and type=changed in the rects), in this case having the type in both the general structure and in each rects seems redundant and leads to possible data inconsistencies. ... I would rather go to solution 1, but I'm not sure if having more than one hint in the side data (with different subtypes) is possible and wanted. If this is the case, probably the best solution would be to store more than one AVVideoHint (each one with an individual type and list of rects) in the single side data object. > +} AVVideoHint; > + > +static av_always_inline AVVideoRect* > +av_video_hint_rects(const AVVideoHint *hints) > +{ > + return (AVVideoRect *)((uint8_t *)hints + hints->rect_offset); > +} > + > +static av_always_inline AVVideoRect* > +av_video_hint_get_rect(const AVVideoHint *hints, size_t idx) > +{ > + return (AVVideoRect *)((uint8_t *)hints + hints->rect_offset + idx * hints->rect_size); > +} > + > +/** > + * Allocate memory for the AVVideoHint struct along with an nb_rects-sized > + * arrays of AVVideoRect. > + * > + * The side data contains a list of rectangles for the portions of the frame > + * which changed from the last encoded one (and the remainder are assumed to be > + * changed), or, alternately (depending on the type parameter) the unchanged > + * ones (and the remanining ones are those which changed). > + * Macroblocks will thus be hinted either to be P_SKIP-ped or go through the > + * regular encoding procedure. > + * > + * It's responsibility of the caller to fill the AVRects accordingly, and to set > + * the proper AVVideoHintType field. > + * > + * @param out_size if non-NULL, the size in bytes of the resulting data array is > + * written here > + * > + * @return newly allocated AVVideoHint struct (must be freed by the caller using > + * av_free()) on success, NULL on memory allocation failure > + */ > +AVVideoHint *av_video_hint_alloc(size_t nb_rects, > + size_t *out_size); > +/** > + * Same as av_video_hint_alloc(), except newly-allocated AVVideoHint is attached > + * as side data of type AV_FRAME_DATA_VIDEO_HINT_INFO to frame. > + */ > +AVVideoHint *av_video_hint_create_side_data(AVFrame *frame, > + size_t nb_rects); > + > + > +#endif /* AVUTIL_VIDEO_HINT_H */ > -- > 2.34.1 > > _______________________________________________ > 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". _______________________________________________ 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".