On Sat, Jul 8, 2023 at 12:30 AM James Almer <jamr...@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 <jamr...@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@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". > _______________________________________________ > 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". > Hi James, thank you for the details. Ok, understood, I will change to using CBS. This change may take some time. _______________________________________________ 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".