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 ESMTPS id D5EEC4D6C5 for ; Sat, 22 Feb 2025 17:36:49 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id DA69568C61C; Sat, 22 Feb 2025 19:36:43 +0200 (EET) Received: from sender2-op-o18.zoho.eu (sender2-op-o18.zoho.eu [136.143.171.18]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 6D9B368C595 for ; Sat, 22 Feb 2025 19:36:37 +0200 (EET) ARC-Seal: i=1; a=rsa-sha256; t=1740245794; cv=none; d=zohomail.eu; s=zohoarc; b=S2AiNcDhQCymnaHyHVx29hMpBD9wxnBKQVPS1toBEdAm8RW7juQGdhhFRMBg6hPsQb+NxEYnTJKmRkbo270vnGbhsrvAxdmloveLxbV39I7SAdidrAUkGccwN80kj/mtRRFlWJbTuh0tq6A3w+IRpIFOPCgWuQH5uOY7HmduGY4= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.eu; s=zohoarc; t=1740245794; h=Content-Type:Content-Transfer-Encoding:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To:Cc; bh=9G8VRuNdwNnDM1TVq8i2Wwf9ACOITAGoGzmzDBzmIyE=; b=fMZGdCHyyzBDCPYIU/Ri8uzc/l5B7ARsCWJivT6Ox2vkcB2NjVww4klIpoagcqmgq9N4oHWzBnAyOx9FmQT1ffQqBhz4xE+f1GdQno1do9ElVAIxpOyw4vASkx6sBgRCUzYXPVts2s60wsRhVBDB0I167IABMX6n1yZutxiAhrk= ARC-Authentication-Results: i=1; mx.zohomail.eu; dkim=pass header.i=frankplowman.com; spf=pass smtp.mailfrom=post@frankplowman.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1740245794; s=zmail; d=frankplowman.com; i=post@frankplowman.com; h=Message-ID:Date:Date:MIME-Version:Subject:Subject:To:To:References:From:From:In-Reply-To:Content-Type:Content-Transfer-Encoding:Message-Id:Reply-To:Cc; bh=9G8VRuNdwNnDM1TVq8i2Wwf9ACOITAGoGzmzDBzmIyE=; b=RXYLBok9mzgTCMItQ0Pfb7VwZ+t9Rv9ayWMpMV2G4NVvw/iQ7Lf0TnaD4n1PyYbo +EaZEXaPrybi1inqC9vy2gwsPuiKgaBaDidYvRaX3ViR+KWFBAiWGLmy6/XelPKnQ10 pwoq7lEA7yDNXNw0sPVb32hCAUyvJ2YR8gQxx/Us= Received: by mx.zoho.eu with SMTPS id 1740245792764354.1314713402154; Sat, 22 Feb 2025 18:36:32 +0100 (CET) Message-ID: Date: Sat, 22 Feb 2025 17:36:32 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-GB To: ffmpeg-devel@ffmpeg.org References: <20250222075155.504540-1-nuomi2021@gmail.com> <20250222075155.504540-2-nuomi2021@gmail.com> From: Frank Plowman In-Reply-To: <20250222075155.504540-2-nuomi2021@gmail.com> X-ZohoMailClient: External Subject: Re: [FFmpeg-devel] [PATCH v3 2/2] lavc/vvc: Ensure subpictures don't overlap 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 22/02/2025 07:51, Nuo Mi wrote: > From: Frank Plowman > > This is essentially a re-implementation of > https://patchwork.ffmpeg.org/project/ffmpeg/patch/20241005223955.54158-1-post@frankplowman.com/ > > That patch was not applied last time. Instead we opted to identify > issues which could be caused by invalid subpicture layouts and remedy > those issues where they manifest, either through error detection or code > hardening. This was primarily implemented in the set > https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13381. > > This has worked to some degree, however issues with subpicture layouts > continue to crop up from the fuzzer and I've fixed a number of bugs > related to subpicture layouts since then. I think it's best to return > to the initial plan and simply check if the subpicture layout is valid > initially. > > This implementation is also lighter than the first time -- by doing a > bit more logic in pps_subpic_less_than_one_tile_slice, we are able to > store a tile_in_subpic map rather than a ctu_in_subpic map. This > reduces the size of the map to the point it becomes possible to allocate > it on the stack. Similar to 8bd66a8c9587af61c7b46558be3c4ee317c1af5a, > the layout is also validated in the slice map construction code, rather > than in the CBS, which avoids duplicating some logic. > > Signed-off-by: Frank Plowman > --- > libavcodec/vvc/ps.c | 55 ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 45 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/vvc/ps.c b/libavcodec/vvc/ps.c > index d8794277d2..9b8af5524b 100644 > --- a/libavcodec/vvc/ps.c > +++ b/libavcodec/vvc/ps.c > @@ -450,25 +450,50 @@ static void subpic_tiles(int *tile_x, int *tile_y, int *tile_x_end, int *tile_y_ > (*tile_y_end)++; > } > > -static void pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS *sps, const int i, const int tx, const int ty, int *off) > +static bool mark_tile_as_used(bool *tile_in_subpic, const int tx, const int ty, const int tile_columns) > { > + const size_t tile_idx = ty * tile_columns + tx; > + if (tile_in_subpic[tile_idx]) { > + /* the tile is covered by other subpictures */ > + return false; > + } > + tile_in_subpic[tile_idx] = true; > + return true; > +} > + > +static int pps_subpic_less_than_one_tile_slice(VVCPPS *pps, const VVCSPS *sps, const int i, const int tx, const int ty, int *off, bool *tile_in_subpic) > +{ > + const int subpic_bottom = sps->r->sps_subpic_ctu_top_left_y[i] + sps->r->sps_subpic_height_minus1[i]; > + const int tile_bottom = pps->row_bd[ty] + pps->r->row_height_val[ty] - 1; > + const bool is_final_subpic_in_tile = subpic_bottom == tile_bottom; > + > + if (is_final_subpic_in_tile && !mark_tile_as_used(tile_in_subpic, tx, ty, pps->r->num_tile_columns)) > + return AVERROR_INVALIDDATA; > + > pps->num_ctus_in_slice[i] = pps_add_ctus(pps, off, > sps->r->sps_subpic_ctu_top_left_x[i], sps->r->sps_subpic_ctu_top_left_y[i], > sps->r->sps_subpic_width_minus1[i] + 1, sps->r->sps_subpic_height_minus1[i] + 1); > + > + return 0; > } > > -static void pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int tile_x, const int tile_y, const int x_end, const int y_end, const int i, int *off) > +static int pps_subpic_one_or_more_tiles_slice(VVCPPS *pps, const int tile_x, const int tile_y, const int x_end, const int y_end, > + const int i, int *off, bool *tile_in_subpic) > { > for (int ty = tile_y; ty < y_end; ty++) { > for (int tx = tile_x; tx < x_end; tx++) { > + if (!mark_tile_as_used(tile_in_subpic, tx, ty, pps->r->num_tile_columns)) > + return AVERROR_INVALIDDATA; > + > pps->num_ctus_in_slice[i] += pps_add_ctus(pps, off, > pps->col_bd[tx], pps->row_bd[ty], > pps->r->col_width_val[tx], pps->r->row_height_val[ty]); > } > } > + return 0; > } > > -static void pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off) > +static int pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *off, bool *tile_in_subpic) > { > int tx, ty, x_end, y_end; > > @@ -477,19 +502,30 @@ static void pps_subpic_slice(VVCPPS *pps, const VVCSPS *sps, const int i, int *o > > subpic_tiles(&tx, &ty, &x_end, &y_end, sps, pps, i); > if (ty + 1 == y_end && sps->r->sps_subpic_height_minus1[i] + 1 < pps->r->row_height_val[ty]) > - pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off); > + return pps_subpic_less_than_one_tile_slice(pps, sps, i, tx, ty, off, tile_in_subpic); > else > - pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off); > + return pps_subpic_one_or_more_tiles_slice(pps, tx, ty, x_end, y_end, i, off, tile_in_subpic); > } > > -static void pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps, int *off) > +static int pps_single_slice_per_subpic(VVCPPS *pps, const VVCSPS *sps, int *off) > { > if (!sps->r->sps_subpic_info_present_flag) { > pps_single_slice_picture(pps, off); > } else { > - for (int i = 0; i < pps->r->pps_num_slices_in_pic_minus1 + 1; i++) > - pps_subpic_slice(pps, sps, i, off); > + bool tile_in_subpic[VVC_MAX_TILES_PER_AU] = {0}; > + for (int i = 0; i < pps->r->pps_num_slices_in_pic_minus1 + 1; i++) { > + const int ret = pps_subpic_slice(pps, sps, i, off, tile_in_subpic); > + if (ret < 0) > + return ret; > + } > + > + // We only use tile_in_subpic to check that the subpictures don't overlap > + // here; we don't use tile_in_subpic to check that the subpictures cover > + // every tile. It is possible to avoid doing this work here because the > + // covering property of subpictures is already guaranteed by the mechanisms > + // which check every CTU belongs to a slice. > } > + return 0; > } > > static int pps_one_tile_slices(VVCPPS *pps, const int tile_idx, int i, int *off) > @@ -540,8 +576,7 @@ static int pps_rect_slice(VVCPPS *pps, const VVCSPS *sps) > int tile_idx = 0, off = 0; > > if (r->pps_single_slice_per_subpic_flag) { > - pps_single_slice_per_subpic(pps, sps, &off); > - return 0; > + return pps_single_slice_per_subpic(pps, sps, &off); > } > > for (int i = 0; i < r->pps_num_slices_in_pic_minus1 + 1; i++) { Thanks, LGTM! -- Frank _______________________________________________ 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".