Skip to content

Conversation

@QSXW
Copy link
Collaborator

@QSXW QSXW commented Jun 3, 2025

The fix for DUAL_CHROMA_TREE coding unit was lost in our rebased commits. Please help review 0f9b47a

@QSXW QSXW requested a review from nuomi2021 June 3, 2025 18:46
@QSXW QSXW changed the base branch from up to main June 3, 2025 19:14
@QSXW QSXW changed the base branch from main to up June 3, 2025 19:14
@nuomi2021
Copy link
Member

The fix for DUAL_CHROMA_TREE coding unit was lost in our rebased commits. Please help review 0f9b47a

thank you for the patch

For which specific clip are you seeing the issue? All ACT clips pass locally with the upstream code.

like:
ACTPIC_A_Huawei_3.bit ACTPIC_B_Huawei_3.bit ACTPIC_C_Huawei_3.bit ACT_A_Kwai_3.bit ACT_B_Kwai_3.bit

@QSXW
Copy link
Collaborator Author

QSXW commented Jun 8, 2025

The fix for DUAL_CHROMA_TREE coding unit was lost in our rebased commits. Please help review 0f9b47a

thank you for the patch

For which specific clip are you seeing the issue? All ACT clips pass locally with the upstream code.

like: ACTPIC_A_Huawei_3.bit ACTPIC_B_Huawei_3.bit ACTPIC_C_Huawei_3.bit ACT_A_Kwai_3.bit ACT_B_Kwai_3.bit

It's the one encoded by myself. See #224 (comment)

We should use the width and height of the start component. When the cu is DUAL_CHROMA_TREE, the start component is the first transform block, so we need to use the tb width and height.

@QSXW QSXW force-pushed the fate/add_plt_act_field branch 2 times, most recently from d5d5f5f to 44b6d39 Compare June 8, 2025 16:12
@nuomi2021
Copy link
Member

It's the one encoded by myself. See #224 (comment)

We should use the width and height of the start component. When the cu is DUAL_CHROMA_TREE, the start component is the first transform block, so we need to use the tb width and height.

could you share the DUAL_CHROMA_TREE clip with me?
Thank you

@nuomi2021
Copy link
Member

nuomi2021 commented Jun 9, 2025

It's better to remove the company name from the clip, just like the other FATE clips.

@QSXW
Copy link
Collaborator Author

QSXW commented Jun 9, 2025

It's the one encoded by myself. See #224 (comment)
We should use the width and height of the start component. When the cu is DUAL_CHROMA_TREE, the start component is the first transform block, so we need to use the tb width and height.

could you share the DUAL_CHROMA_TREE clip with me? Thank you

Sure. https://github.com/user-attachments/assets/91fce515-47bf-45e6-bc32-4473eba2260f

To upload the clip, I changed the file extension from bit to mp4. You need to change it back.

Open the link and use Ctrl + S to download the clip.

@QSXW QSXW force-pushed the fate/add_plt_act_field branch from 44b6d39 to 8ebea9f Compare June 10, 2025 19:48
const int scale = ff_vvc_palette_derive_scale(lc, tu, tb);
const int hs = sps->hshift[c];
const int vs = sps->vshift[c];
const int hs = tu->tbs[0].c_idx ? 0 : sps->hshift[c_idx];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about defining a start_index? Then this can be simplified to sps->hshift[c_idx] - sps->hshift[start_index], which makes it more readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

#if HAVE_BIGENDIAN
if (c->type != HASH_CHECKSUM) {
if (ps && !c->buf) {
av_fast_malloc(&c->buf, &c->buf_size,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to free this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes update.

#if HAVE_BIGENDIAN
if (c->type != HASH_CHECKSUM) {
if (ps && !c->buf) {
av_fast_malloc(&c->buf, &c->buf_size,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the frame resolution or ps changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

Makes find output consistent.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
@QSXW QSXW force-pushed the fate/add_plt_act_field branch from 8ebea9f to 4a0437d Compare August 4, 2025 22:20
@QSXW
Copy link
Collaborator Author

QSXW commented Aug 4, 2025

@nuomi2021 Let's get this to mainline. Should I send it to the mailing list?

kasper93 and others added 13 commits August 5, 2025 03:27
Fixes use-of-uninitialized-value when bp.len == 0.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Commit f566032 added frame validation.
Since then this decoder has been failing validation of sample rate
value.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Commit f566032 added frame validation.
Since then this decoder has been failing validation of sample rate
value.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Commit f566032 added frame validation.
Since then this decoder has been failing validation of sample rate
value.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Commit f566032 added frame validation.
Since then this decoder has been failing validation of sample rate
value.

Found by OSS-Fuzz.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Printing dummy logs during fuzzing can significantly slow the process
and blow the size of logs, making them both unredable and huge.

Keep the loggging commented-out for easy local restore if needed.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
The check to return on EOF should not be inside a block that will not be entered after reaching EOF.
Should fix "libavcodec/bytestream.h:144:27: runtime error: applying zero offset to null pointer".

Signed-off-by: James Almer <jamrial@gmail.com>
Add allow extension name cmfa and cmfv test, this testcase only
cover fragment mp4 named cmfa.

ticket description in ticket/11526
This makes it possible to apply Adobe .cube files to inputs.
NVIDIA's support for it is a disaster.
Of no benefit to other vendors.

NVIDIA are working on fixing it, but it may take time.
This makes left_bits return useful data rather than overflowing, and
also saves some 64-bit integer operations, which is still always a plus sadly.
russelltg and others added 10 commits August 5, 2025 23:53
Previously, it was assumed that `drmFormatModifierPlaneCount` was one
for all modifiers when exporting, which is not always the case, in
particular for AMD GPUs and maybe others.

Fetch the number of memory planes and fill the structs appropriately in this situation.

The encoded stream is still bad in the case whre modifers are involved,
but I think this patch still stands on its own and I suspect that may be a driver bug.

A potential improvement that could be make is to cache the format
information, so we can avoid the two GetPhysicalDeviceFormatProperties2
calls for each export, as well as the allocation. I doubt this is very
expensive, but seemed worth noting.

v2 changes: query the format properties with the test image created in
`vulkan_frames_init` to avoid allocating space for the query during
export

Signed-off-by: Russell Greene <russellgreene8@gmail.com>
There was implicit assumption that the $TMPC file is empty when doing
--cpu=host checks. This breaks if any check is done before that.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
This avoids adding flags that cl.exe doesn't understand.

Fixes cases where external libraries pkg-config file adds `-L` to the
cflags, strip it before passing to cl.exe.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
This is important for feature checking to work correctly.

It can happen that an unrecognized flag passes the compile test with
only a warning, while failing in preprocessor-only check with an error.
This causes all test_cpp calls to fail and silently produces arguably
broken MSVC builds. Also, all check_* functions don't work as expected,
because they assume the check passed, even though there was a warning.

Additionally, this brings the behavior in line with GCC/Clang based
builds, failing early on unrecognized flags instead of silently
continuing with warnings in the log.

The /options:strict option is available starting in Visual Studio 2022
version 17.0. Because of that, we cannot use check_cflags alone, as it
would add this flag for older MSVC versions and produce warnings. So, we
need to manually perform a version check. A bit of a chicken and egg
problem.

Perform the version check before adding extra flags from the user to
ensure we don't silently fail the preprocessor check due to invalid
flags on older MSVC versions. Note that behavior differs depending on
whether we are compiling or only preprocessing.

This fixes silent different between handling:

`cl.exe -P foo c.c`
    c1: fatal error C1083: Cannot open source file: 'foo': No such file
    or directory

`cl.exe -c foo c.c`
    cl : Command line warning D9024 : unrecognized source file type
    'foo', object file assumed

Where -P fails, while -c throws warnings only. Of course `foo` is
completely bogus here, but depends on the flags or configuration this
may be unsupported argument. Or even some converted path from MSYS when
run inside it. The objective is to always error out instead of silently
hiding this.

Use check_cflags even after the _MSC_FULL_VER check, for non-MSVC
compilers. For example Clang-CL impersonate MSVC, but does not support
-options:strict flag currently.

Signed-off-by: Kacper Michajłow <kasper93@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
…hitectures

This changes configure to stop disabling -ftree-vectorize on
GCC versions 13 and newer, on major architectures.

Background:
- Original `-fno-tree-vectorize` was added in 2009 in commit
  973859f to avoid compiler errors.
- Re-enabled in 2016 in commit cb8646a but caused failures due
  to inline CABAC assembly issues and was disabled again in
  fd6dbc5.
- Commit 182663a in 2023 fixed the inline CABAC assembly issues.
- Recent versions of GCC, in particular 13 and newer, seem to
  generally work reliably with respect to vectorization, although bugs
  have been observed on Loongarch.

Cautiously allow the GCC default of having vectorization enabled,
on major architectures where we expect to see enough testing. If
further issues are observed, they should be reported and noted here in
configure, so the workarounds can be scoped and version limited.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
… it's not already in place

If the first Sample references the first stsd entry, then setting it here is
redundant.

Signed-off-by: James Almer <jamrial@gmail.com>
Introduced by 307983b

Use the following command line to reproduce the issue:
./configure --toolchain=msvc --disable-asm --enable-ffmpeg \
--disable-everything --enable-decoder=vvc --enable-parser=vvc \
--enable-demuxer='vvc,mpegts' --enable-protocol='file,pipe' \
--enable-encoder='rawvideo,wrapped_avframe' \
--enable-muxer='rawvideo,md5,null'

Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
@QSXW QSXW force-pushed the fate/add_plt_act_field branch from 4a0437d to bae25c8 Compare August 6, 2025 18:53
QSXW and others added 7 commits August 7, 2025 03:51
This commit fixed decoding the DUAL_TREE_CHROMA palette coding unit

Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
…rrect

It's helpful for developers and the same as the hevcdec.

Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
This commit added 10b422_L_5 for testing palette mode.

Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
Signed-off-by: Wu Jianhua <toqsxw@outlook.com>
@QSXW QSXW force-pushed the fate/add_plt_act_field branch from bae25c8 to ed06eda Compare August 6, 2025 19:51
@nuomi2021
Copy link
Member

+1. Better to group patches with related functions and send them to the mailing list separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.