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 3C8F748DAD for ; Fri, 26 Apr 2024 12:36:54 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EFF4F68D431; Fri, 26 Apr 2024 15:36:51 +0300 (EEST) Received: from haasn.dev (haasn.dev [78.46.187.166]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0E6BA68D2B5 for ; Fri, 26 Apr 2024 15:36:45 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=haasn.xyz; s=mail; t=1714135004; bh=F0LYT0YqRN/zhdxBoAQez6I6IXR5BxWSOOmB3LQi7hc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=MEJIHoH2Qav67f2NqhQK7JVjl0R8H1fiYLG8qXJXZdJa4SuPDkvlDxtc8RDjy659/ +slJyGxz6e601AKcV2vNafO4AmFISrWL2ufhMmsl0Cbd6j5D4ABOzcRKm9J2PbRmNi 0eGW/t0HplRLC7YJ/w6EIshZuWuOjwD0txc3LRMs= Received: from haasn.dev (unknown [10.30.0.2]) by haasn.dev (Postfix) with ESMTP id C8F9940044; Fri, 26 Apr 2024 14:36:44 +0200 (CEST) Date: Fri, 26 Apr 2024 14:36:44 +0200 Message-ID: <20240426143644.GB20567@haasn.xyz> From: Niklas Haas To: ffmpeg-devel@ffmpeg.org In-Reply-To: <20240426122803.19967-1-ffmpeg@haasn.xyz> References: <20240426122803.19967-1-ffmpeg@haasn.xyz> MIME-Version: 1.0 Content-Disposition: inline Subject: Re: [FFmpeg-devel] [PATCH 1/6] avutil/frame: add av_frame_remove_side_data_changed 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 Cc: Niklas Haas 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 Fri, 26 Apr 2024 14:27:58 +0200 Niklas Haas wrote: > From: Niklas Haas > > Many filters modify certain aspects of frame data, e.g. through resizing > (vf_*scale* family), color volume mapping (vf_lut*, vf_tonemap*), or > possibly others. > > When this happens, we should strip all frame side data that will no > longer be correct/relevant after the operation. For example, changing > the image size should invalidate AV_FRAME_DATA_PANSCAN because the crop > window (given in pixels) no longer corresponds to the actual image size. > For another example, tone-mapping filters (e.g. from HDR to SDR) should > strip all of the dynamic HDR related metadata. > > Since there are a lot of similar with basically similar operations, it > make sense to consolidate this stripping logic into a common helper > function. I decided to put it into libavutil as it may be useful for API > users as well, who often have their own internal processing and > filtering. > --- > doc/APIchanges | 3 +++ > libavutil/frame.c | 31 +++++++++++++++++++++++++++++++ > libavutil/frame.h | 14 ++++++++++++++ > libavutil/version.h | 2 +- > 4 files changed, 49 insertions(+), 1 deletion(-) > > diff --git a/doc/APIchanges b/doc/APIchanges > index 0566fcdcc5..26af725528 100644 > --- a/doc/APIchanges > +++ b/doc/APIchanges > @@ -2,6 +2,9 @@ The last version increases of all libraries were on 2024-03-07 > > API changes, most recent first: > > +2024-04-xx - xxxxxxxxxx - lavu 59.17.100 - frame.h > + Add av_frame_remove_side_data_changed(). > + > 2024-04-24 - 8616cfe0890 - lavu 59.16.100 - opt.h > Add AV_OPT_SERIALIZE_SEARCH_CHILDREN. > > diff --git a/libavutil/frame.c b/libavutil/frame.c > index 0775e2abd9..d5482c258e 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -1015,6 +1015,37 @@ void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type) > remove_side_data(&frame->side_data, &frame->nb_side_data, type); > } > > +static const struct { > + enum AVFrameSideDataType type; > + int aspect; > +} side_data_aspects[] = { > + { AV_FRAME_DATA_PANSCAN, AV_FRAME_CHANGED_SIZE }, > + { AV_FRAME_DATA_MOTION_VECTORS, AV_FRAME_CHANGED_SIZE }, > + { AV_FRAME_DATA_MASTERING_DISPLAY_METADATA, AV_FRAME_CHANGED_COLOR_VOLUME }, > + > + { AV_FRAME_DATA_SPHERICAL, AV_FRAME_CHANGED_SIZE }, > + { AV_FRAME_DATA_CONTENT_LIGHT_LEVEL, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_ICC_PROFILE, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_DYNAMIC_HDR_PLUS, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_REGIONS_OF_INTEREST, AV_FRAME_CHANGED_SIZE }, > + { AV_FRAME_DATA_DETECTION_BBOXES, AV_FRAME_CHANGED_SIZE }, > + { AV_FRAME_DATA_DOVI_RPU_BUFFER, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_DOVI_METADATA, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_DYNAMIC_HDR_VIVID, AV_FRAME_CHANGED_COLOR_VOLUME }, > + { AV_FRAME_DATA_VIDEO_HINT, AV_FRAME_CHANGED_SIZE }, > +}; > + > +void av_frame_remove_side_data_changed(AVFrame *frame, int changed_aspects) > +{ > + if (!changed_aspects) > + return; > + > + for (int i = 0; i < FF_ARRAY_ELEMS(side_data_aspects); i++) { > + if (changed_aspects & side_data_aspects[i].aspect) > + av_frame_remove_side_data(frame, side_data_aspects[i].type); > + } > +} > + > const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType type) > { > unsigned t = type; > diff --git a/libavutil/frame.h b/libavutil/frame.h > index 60bb966f8b..7e07ecf91f 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -983,6 +983,20 @@ AVFrameSideData *av_frame_get_side_data(const AVFrame *frame, > */ > void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type); > > +/** > + * Flags for stripping side data based on changed aspects. > + */ > +enum { > + /* Video only */ > + AV_FRAME_CHANGED_SIZE = 1 << 0, ///< changed dimensions / crop > + AV_FRAME_CHANGED_COLOR_VOLUME = 1 << 1, ///< changed color volume > +}; We almost surely also want to do something similar for audio-related metadata. I'm not too familiar with audio, does something like this seem right? On changing channel layout: - strip AV_FRAME_DATA_MATRIXENCODING - strip AV_FRAME_DATA_DOWNMIX_INFO - strip AV_FRAME_DATA_REPLAYGAIN (maybe?) On changing sample rate: - strip AV_FRAME_DATA_SKIP_SAMPLES Anything else? What about S12M_TIMECODE / GOP_TIMECODE? Shouldn't we strip these if the pts / timebase changes? > + > +/** > + * Remove all relevant side data after a corresponding change to the frame > + * data, based on the given bitset of frame aspects. > + */ > +void av_frame_remove_side_data_changed(AVFrame *frame, int changed_aspects); > > /** > * Flags for frame cropping. > diff --git a/libavutil/version.h b/libavutil/version.h > index ea289c406f..3b5a2e7aaa 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -79,7 +79,7 @@ > */ > > #define LIBAVUTIL_VERSION_MAJOR 59 > -#define LIBAVUTIL_VERSION_MINOR 16 > +#define LIBAVUTIL_VERSION_MINOR 17 > #define LIBAVUTIL_VERSION_MICRO 100 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > -- > 2.44.0 > _______________________________________________ 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".