Crash when using 4 tiles for 1080p 4:2:2 input (but not with 8 tiles) #2212

gnafuthegreat · 2020-03-30T13:14:38Z

Description:
When encoding 1080p 4:2:2 video (even a single black frame) with 4 tiles, rav1e crashes. With 8 tiles, the encoding finishes normally.

Command line:
rav1e -o /dev/null --tiles 4 black-frame.y4m

Result with debug build and RUSTBACKTRACE=full:
rav1e-422-crash.txt

Backtrace (most recent call first):
  File "<unknown>", line 0, in core::panicking::panic
  File "/var/home/gideon/rav1e/src/lrf.rs", line 1416, in rav1e::lrf::RestorationState::new
    uv_unit_log2 - uv_sb_v_log2,
  File "/var/home/gideon/rav1e/src/encoder.rs", line 384, in rav1e::encoder::FrameState<T>::new_with_frame
    let rs = RestorationState::new(fi, &frame);
  File "/var/home/gideon/rav1e/src/api/internal.rs", line 215, in rav1e::api::internal::FrameData<T>::new
    let fs = FrameState::new_with_frame(&fi, frame);
  File "/var/home/gideon/rav1e/src/api/internal.rs", line 426, in rav1e::api::internal::ContextInner<T>::set_frame_properties
    self.frame_data.insert(output_frameno, FrameData::new(fi, frame.clone()));
  File "/var/home/gideon/rav1e/src/api/internal.rs", line 744, in rav1e::api::internal::ContextInner<T>::compute_frame_invariants
    while self.set_frame_properties(self.next_lookahead_output_frameno).is_ok()
  File "/var/home/gideon/rav1e/src/api/internal.rs", line 348, in rav1e::api::internal::ContextInner<T>::send_frame
    self.compute_frame_invariants();
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/thread_pool/mod.rs", line 110, in rayon_core::thread_pool::ThreadPool::install::{{closure}}
    self.registry.in_worker(|_, _| op())
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 447, in rayon_core::registry::Registry::in_worker_cold::{{closure}}::{{closure}}
    op(&*worker_thread, true)
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/job.rs", line 113, in <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute::call::{{closure}}
    move || func(true)
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panic.rs", line 318, in <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panicking.rs", line 305, in std::panicking::try::do_call
  File "<unknown>", line 0, in __rust_maybe_catch_panic
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panicking.rs", line 281, in std::panicking::try
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panic.rs", line 394, in std::panic::catch_unwind
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/unwind.rs", line 17, in rayon_core::unwind::halt_unwinding
    panic::catch_unwind(AssertUnwindSafe(func))
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/job.rs", line 119, in <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute
    (*this.result.get()) = match unwind::halt_unwinding(call(func)) {
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 681, in rayon_core::registry::WorkerThread::execute
    job.execute();
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 665, in rayon_core::registry::WorkerThread::wait_until_cold
    self.execute(job);
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 639, in rayon_core::registry::WorkerThread::wait_until
    self.wait_until_cold(latch);
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 759, in rayon_core::registry::main_loop
    worker_thread.wait_until(&registry.terminate_latch);
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 56, in rayon_core::registry::ThreadBuilder::run
    unsafe { main_loop(self.worker, self.registry, self.index) }
  File "/home/gideon/.cargo/registry/src/github.jparrowsec.cn-1ecc6299db9ec823/rayon-core-1.7.0/src/registry.rs", line 101, in <rayon_core::registry::DefaultSpawn as rayon_core::registry::ThreadSpawn>::spawn::{{closure}}
    b.spawn(|| thread.run())?;
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/sys_common/backtrace.rs", line 129, in std::sys_common::backtrace::__rust_begin_short_backtrace
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/thread/mod.rs", line 475, in std::thread::Builder::spawn_unchecked::{{closure}}::{{closure}}
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panic.rs", line 318, in <std::panic::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panicking.rs", line 305, in std::panicking::try::do_call
  File "<unknown>", line 0, in __rust_maybe_catch_panic
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panicking.rs", line 281, in std::panicking::try
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/panic.rs", line 394, in std::panic::catch_unwind
  File "/builddir/build/BUILD/rustc-1.42.0-src/src/libstd/thread/mod.rs", line 474, in std::thread::Builder::spawn_unchecked::{{closure}}
  File "<unknown>", line 0, in <alloc::boxed::Box<F> as core::ops::function::FnOnce<A>>::call_once
  File "<unknown>", line 0, in std::sys::unix::thread::Thread::new::thread_start
  File "<unknown>", line 0, in start_thread
  File "<unknown>", line 0, in clone
  File "<unknown>", line 0, in <unknown>

The application panicked (crashed).
  attempt to subtract with overflow
in /var/home/gideon/rav1e/src/lrf.rs, line 1416
thread: <unnamed>

The text was updated successfully, but these errors were encountered:

xiphmont · 2020-04-01T20:10:07Z

Reproduced. Working on it.

xiphmont · 2020-04-01T20:56:10Z

Update: Bug is due to restoration filter being handed a tiling setup it cannot honor. Tiling is selecting a tile width of 15 SBs, and in 4:2:2 LRU can't do 32x32, so it would be forced to have a non-integer horizontal number of LRUs per tile.

Restoration needs to check and flag this case for debugging/abort purposes. Tiling needs to add a check to enforce even number of horizontal SBs per tile in the 4:2:2 case.

xiphmont · 2020-04-02T21:58:58Z

aaand that has exposed something else. Still working on it :0)

tdaede · 2020-04-16T21:27:16Z

Can we disable enough filters on 4:2:2 that it doesn't crash?

xiphmont · 2020-04-16T21:55:53Z

Sadly, I don't think so-- the problem is that Tiling is partially implemented in terms of the restoration filter framework and piggybacks on some of its infrastructure. You can 'disable' reconstruction, but that just means the filter doesn't run. It's not the filter that's crashing, it's the overall LRU accounting, which tiling uses. This is part of the stack of oopsie that hadn't been getting tested and turns out not to have worked in a while :-(

…

On Thu, Apr 16, 2020 at 5:27 PM Thomas Daede ***@***.***> wrote: Can we disable enough filters on 4:2:2 that it doesn't crash? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#2212 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACWMHEEILHLBCZGY7LLAXADRM5Z4HANCNFSM4LWSOCQA> .

barrbrain · 2020-04-17T09:03:14Z

Another data point from fuzzing. I think LRF should be disabled because of the height but we crash in RestorationState::new() anyway.

DecodeTestParameters {
    w: 260,
    h: 16,
    chroma_sampling: Cs422,
    tile_cols_log2: 1,
    tile_rows_log2: 1,
}

        DecodeTestParameters {
            w: 260,
            h: 16,
            speed: 10,
            q: 0,
            limit: 1,
            bit_depth: 8,
            chroma_sampling: Cs422,
            min_keyint: 3,
            max_keyint: 3,
            switch_frame_interval: 0,
            low_latency: true,
            error_resilient: true,
            bitrate: 65535,
            tile_cols_log2: 1,
            tile_rows_log2: 1,
            still_picture: false,
        }

When doing loop filter RDO inline with the rest of the tile coding, LRUs must align to tile boundaries. An unexpected corner case means that chroma LRUs must have an even superblock width in 4:2:2 video, as LRUs must always be square. As a result, that means tiles must also have an even superblock width. As tile width must be adjusted in this case, it also means we can't use the spec's 'tile uniform spacing' mode, which would produce odd superblock width tiles in, eg, 1080p 4:2:2 video. This patch also implements explicit per-tile sizing the the frame OBU header.

lu-zero assigned xiphmont Mar 30, 2020

lu-zero mentioned this issue Apr 8, 2020

Enable CDEF and restoration filters for 4:2:2 #2224

Merged

negge added the bug label Apr 15, 2020

barrbrain mentioned this issue May 11, 2020

Full 4:2:2/4:4:4 support #970

Open

5 tasks

barrbrain closed this as completed May 15, 2020

xiphmont mentioned this issue May 22, 2020

Corrupt bitstream: multi-pass encoding #2311

Closed

xiphmont mentioned this issue Jun 5, 2020

Fix for issue 2311: Corrupt bitstream: multi-pass encoding #2371

Merged

vibhoothi mentioned this issue Sep 15, 2020

0.4.0 preparation #2544

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Crash when using 4 tiles for 1080p 4:2:2 input (but not with 8 tiles) #2212

Crash when using 4 tiles for 1080p 4:2:2 input (but not with 8 tiles) #2212

gnafuthegreat commented Mar 30, 2020 •

edited by lu-zero

Loading

xiphmont commented Apr 1, 2020

xiphmont commented Apr 1, 2020 •

edited

Loading

xiphmont commented Apr 2, 2020

tdaede commented Apr 16, 2020

xiphmont commented Apr 16, 2020 via email

barrbrain commented Apr 17, 2020

Crash when using 4 tiles for 1080p 4:2:2 input (but not with 8 tiles) #2212

Crash when using 4 tiles for 1080p 4:2:2 input (but not with 8 tiles) #2212

Comments

gnafuthegreat commented Mar 30, 2020 • edited by lu-zero Loading

xiphmont commented Apr 1, 2020

xiphmont commented Apr 1, 2020 • edited Loading

xiphmont commented Apr 2, 2020

tdaede commented Apr 16, 2020

xiphmont commented Apr 16, 2020 via email

barrbrain commented Apr 17, 2020

gnafuthegreat commented Mar 30, 2020 •

edited by lu-zero

Loading

xiphmont commented Apr 1, 2020 •

edited

Loading