Thank you for your reply. On 14/10/2024 16:16, Nuo Mi wrote: > On Mon, Oct 14, 2024 at 3:14 AM Frank Plowman <p...@frankplowman.com> wrote: > >> On 13/10/2024 05:43, Nuo Mi wrote: >>> On Sun, Oct 6, 2024 at 6:49 AM Frank Plowman <p...@frankplowman.com> >> wrote: >>> >>>> H.266 (V3) section 6.3.3 dictates that the division of the picture into >>>> subpictures must be exhaustive and mutually exclusive, i.e. that each >>>> CTU "belongs to" one and only one subpicture. In most cases this is >>>> guaranteed by the syntax, but in the case sps_subpic_same_size_flag=0, >>>> we must check this is true ourselves. >>>> >>>> Signed-off-by: Frank Plowman <p...@frankplowman.com> >>>> --- >>>> libavcodec/cbs_h266_syntax_template.c | 46 +++++++++++++++++++++++++-- >>>> 1 file changed, 44 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/libavcodec/cbs_h266_syntax_template.c >>>> b/libavcodec/cbs_h266_syntax_template.c >>>> index b4165b43b3..822ee26f46 100644 >>>> --- a/libavcodec/cbs_h266_syntax_template.c >>>> +++ b/libavcodec/cbs_h266_syntax_template.c >>>> @@ -1191,7 +1191,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, >>>> RWContext *rw, >>>> win_left_edge_ctus > >>>> current->sps_subpic_ctu_top_left_x[i] >>>> ? win_left_edge_ctus - >>>> current->sps_subpic_ctu_top_left_x[i] >>>> : 0, >>>> - MAX_UINT_BITS(wlen), 1, i); >>>> + tmp_width_val - >>>> current->sps_subpic_ctu_top_left_x[i] - 1, 1, i); >>>> } else { >>>> infer(sps_subpic_width_minus1[i], >>>> tmp_width_val - >>>> @@ -1208,7 +1208,7 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, >>>> RWContext *rw, >>>> win_top_edge_ctus > >>>> current->sps_subpic_ctu_top_left_y[i] >>>> ? win_top_edge_ctus - >>>> current->sps_subpic_ctu_top_left_y[i] >>>> : 0, >>>> - MAX_UINT_BITS(hlen), 1, i); >>>> + tmp_height_val - >>>> current->sps_subpic_ctu_top_left_y[i] - 1, 1, i); >>>> } else { >>>> infer(sps_subpic_height_minus1[i], >>>> tmp_height_val - >>>> @@ -1242,6 +1242,48 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, >>>> RWContext *rw, >>>> >> infer(sps_loop_filter_across_subpic_enabled_flag[i], >>>> 0); >>>> } >>>> } >>>> + // If the subpic partitioning structure is signalled >>>> explicitly, >>>> + // validate it constitutes an exhaustive and mutually >>>> exclusive >>>> + // coverage of the picture, per 6.3.3. If the partitioning >>>> is not >>>> + // provided explicitly, then it is ensured by the syntax >> and >>>> we need >>>> + // not check. >>>> + if (!current->sps_subpic_same_size_flag) { >>>> + char *ctu_in_subpic = av_mallocz(tmp_width_val * >>>> tmp_height_val); >>>> >>> Thank you for the patch. >>> The slices will cover the entire subpicture without any overlap, and the >>> CTUs will cover the entire slice without any overlap. >>> We will check num_slices_in_subpic[] in FUNC(pps). How about summing all >>> the values in num_slices_in_subpic[] and verifying if it equals >>> sps_num_subpics_minus1 + 1? >> >> This is not sufficient in the case pps_single_slice_per_subpic flag is >> 1. When this flag is 1, the slice layout is the same as the subpicture >> layout and so your suggested condition is always satisfied. In this >> case, we have no guarantees that the slice layout is valid however. >> > We can only determine this after decoding all slice headers. see > https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/vvc/ps.c#L1218
I'm sorry I'm not sure I follow exactly. What can we not determine until decoding slice headers? pps_single_slice_per_subpic is a PPS flag. Also, in the cases I have which have issues with invalid subpic/slice structures, we start hitting bugs and crashes in frame_setup, for example in pps_slice_map, before ff_vvc_decode_sh has been called even. > The task_init_parse function will ensure that a single CTU does not belong > to two slices. Looking at the implementation of task_init_parse, I see it checks a single task object is not initialised multiple times, but the location of this CTU is simply supplied by the caller and already depends on the slice structure (ep->ctu_{start,end}, ctb_addr_in_curr_slice), which may be invalid. As far as I can tell there is nothing here which checks slices don't overlap. Am I missing something? > It might be helpful to add a check in ff_vvc_frame_submit to confirm that > each task (CTU) points to a valid slice (i.e., t->sc is not NULL). I don't think this is possible currently as the way we get tasks is by iterating over those in a SliceContext. If there were CTUs not covered by any slice, the loops in ff_vvc_frame_submit would not see them. > >> >>> >>> + if (!ctu_in_subpic) >>>> + return AVERROR(ENOMEM); >>>> + for (i = 0; i <= current->sps_num_subpics_minus1; i++) >> { >>>> + const unsigned x0 = >>>> current->sps_subpic_ctu_top_left_x[i]; >>>> + const unsigned y0 = >>>> current->sps_subpic_ctu_top_left_y[i]; >>>> + const unsigned w = >>>> current->sps_subpic_width_minus1[i] + 1; >>>> + const unsigned h = >>>> current->sps_subpic_height_minus1[i] + 1; >>>> + av_assert0(x0 + w - 1 < tmp_width_val); >>>> + av_assert0(y0 + h - 1 < tmp_height_val); >>>> + for (unsigned x = x0; x < x0 + w; x++) { >>>> + for (unsigned y = y0; y < y0 + h; y++) { >>>> + const unsigned idx = y * tmp_width_val + x; >>>> + if (ctu_in_subpic[idx]) { >>>> + av_log(ctx->log_ctx, AV_LOG_ERROR, >>>> + "Subpictures overlap.\n"); >>>> + av_freep(&ctu_in_subpic); >>>> + return AVERROR_INVALIDDATA; >>>> + } >>>> + ctu_in_subpic[idx] = 1; >>>> + } >>>> + } >>>> + } >>>> + for (unsigned x = 0; x < tmp_width_val; x++) { >>>> + for (unsigned y = 0; y < tmp_height_val; y++) { >>>> + const unsigned idx = y * tmp_width_val + x; >>>> + if (!ctu_in_subpic[idx]) { >>>> + av_log(ctx->log_ctx, AV_LOG_ERROR, >>>> + "Subpictures do not cover the entire >>>> picture.\n"); >>>> + av_freep(&ctu_in_subpic); >>>> + return AVERROR_INVALIDDATA; >>>> + } >>>> + } >>>> + } >>>> + av_freep(&ctu_in_subpic); >>>> + } >>>> } else { >>>> infer(sps_subpic_ctu_top_left_x[0], 0); >>>> infer(sps_subpic_ctu_top_left_y[0], 0); >>>> -- >>>> 2.46.2 >>>> >> > _______________________________________________ > 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". _______________________________________________ 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".