[FFmpeg-devel] [PATCH v2 03/14] vvcdec: add sps, pps, sh parser

[Nuo Mi]

On Sat, Jul 8, 2023 at 12:30 AM James Almer <jamrial at gmail.com> wrote:

> On 7/7/2023 12:48 PM, Nuo Mi wrote:
> > Hi James,
> > thank you for the review.
> >
> > On Fri, Jul 7, 2023 at 10:28 PM James Almer <jamrial at gmail.com> wrote:
> >
> >> On 7/7/2023 11:05 AM, Nuo Mi wrote:
> >>> ---
> >>>    libavcodec/vvc/Makefile   |    4 +-
> >>>    libavcodec/vvc/vvc_data.c | 3295 +++++++++++++++++++++++++++++++++++
> >>>    libavcodec/vvc/vvc_data.h |   69 +
> >>>    libavcodec/vvc/vvc_ps.c   | 3436
> +++++++++++++++++++++++++++++++++++++
> >>>    libavcodec/vvc/vvc_ps.h   |  828 +++++++++
> >>>    libavcodec/vvc/vvcdec.h   |    3 +
> >>>    6 files changed, 7634 insertions(+), 1 deletion(-)
> >>>    create mode 100644 libavcodec/vvc/vvc_data.c
> >>>    create mode 100644 libavcodec/vvc/vvc_data.h
> >>>    create mode 100644 libavcodec/vvc/vvc_ps.c
> >>>    create mode 100644 libavcodec/vvc/vvc_ps.h
> >>
> >> I thought the plan for v2 was to use CBS? Hence you waiting until it was
> >> pushed.
> >
> > No. The software decoder depends on frame split, I am waiting for it. :)
>
> Why do you need that? You can use the receive_frame() API instead of
> decode(), and only request new packets after you're done with the last
> one you got.
>
> >
> >> CBS can store derived values and state, and you can always pass it a
> >> split AU in a loop if needed.
> >>
> > We have many problems when if we use CBS as a parameter parser
> > 1. The derived value is only needed by the software decoder. like
> > VVCPH.lmcs_fwd_lut and VVCPPS.column_width. Storing it in the CBS
> structure
> > may have performance implications in on other user cases since they never
> > use it. for example, Metadata rewriter
> > 2. CBS strictly follows the spec name, making it hard to write a loop
> like
> > this
> >
> https://github.com/ffvvc/FFmpeg/blob/ae76f43e2e47426544d4dedae1cdc3e16f546458/libavcodec/vvc/vvc_ctu.c#L2136
> .
> > We can use a derived variable, but this makes the variable duplicate the
> > original one.
>
> You can store decoder only derived values in a decoder struct, and have
> CBS store derived values required for header parsing within itself (As
> is the case with all the relevant fields at the end of H266RawPPS), plus
> any other that calculated within CBS itself would simplify things for
> users like the decoder.
>
> > 3. There are so many intermediate names in cbs like abc_minus1. If we use
> > it directly, we may have need to calculate xxx_minus1 + 1 every time. If
> we
> > save abc as a derived value. It will be duplicated again.
>
> Same thing here. After getting a parsed fragment from CBS, you can
> derive all the minus/plus values once and store them in decoder only
> structs, to be used afterwards instead of those in the raw CBS structs.
>
> > 4. Using CBS will not save too much code or logic in some cases. Take the
> > Scaling list as an example, you need a for loop in CBS to read the
> syntax.
> > Then you need a duplicated loop in the software decoder to reconstruct
> the
> > scaling list.
>
> The core gain is to prevent the duplication of all the header parsing
> code, and have a single point of failure and source of bugs.
>
> The h264 and hevc decoders predate CBS, so those were obviously written
> as standalone modules, but this one doesn't, so ideally we should
> prevent duplicating huge bitstream parsing functions a third time.
> Look at AV1, which even if not complete (No software implementation yet)
> has plenty of logic to derive values and tables CBS doesn't need to care
> about, but the hwaccel backends (and the eventual software
> implementation once it's ported from libdav1d) obviously need.
>
> >
> > Again, if Mark agrees and nobody rejects it, I can send out v3 to use it.
> >
> > _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >>
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
Hi James,
thank you for the details.
Ok, understood, I will change to using  CBS. This change may take some
time.