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
The task_init_parse function will ensure that a single CTU does not belong
to two slices.
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).
>
> >
> > + 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".
