Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
gnafuthegreat opened this issue Mar 30, 2020 · 6 comments
Closed

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

gnafuthegreat opened this issue Mar 30, 2020 · 6 comments
Assignees
@xiphmont
Labels
bug

Comments

@gnafuthegreat
Copy link

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

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.com-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.com-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.com-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.com-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.com-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.com-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.com-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.com-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.com-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.com-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.com-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>
@xiphmont
Copy link
Contributor

xiphmont commented Apr 1, 2020

Reproduced. Working on it.

@xiphmont
Copy link
Contributor

xiphmont commented Apr 1, 2020 •
edited
Loading

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
Copy link
Contributor

xiphmont commented Apr 2, 2020

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

@lu-zero lu-zero mentioned this issue Apr 8, 2020
Merged
@negge negge added the bug label Apr 15, 2020
@tdaede
Copy link
Collaborator

tdaede commented Apr 16, 2020

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

@xiphmont
Copy link
Contributor

xiphmont commented Apr 16, 2020 via email

@barrbrain
Copy link
Collaborator

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,
        }

@barrbrain barrbrain mentioned this issue May 11, 2020
Open
5 tasks
xiphmont added a commit to xiphmont/rav1e that referenced this issue May 14, 2020
@xiphmont
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
769ab19 
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.
xiphmont added a commit to xiphmont/rav1e that referenced this issue May 14, 2020
@xiphmont
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
21e432f 
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.
xiphmont added a commit to xiphmont/rav1e that referenced this issue May 14, 2020
@xiphmont
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
842933f 
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.
xiphmont added a commit to xiphmont/rav1e that referenced this issue May 14, 2020
@xiphmont
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
f908efb 
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.
xiphmont added a commit to xiphmont/rav1e that referenced this issue May 14, 2020
@xiphmont
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
f322748 
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.
xiphmont added a commit that referenced this issue May 14, 2020
@xiphmont
Fix for #2212; Crash when using 4 tiles for 1080p 4:2:2 input
51f0eed 
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.
@xiphmont xiphmont mentioned this issue May 22, 2020
Closed
barrbrain pushed a commit to barrbrain/rav1e that referenced this issue May 26, 2020
@xiphmont @barrbrain
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
3e9f191 
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.
barrbrain pushed a commit to barrbrain/rav1e that referenced this issue May 27, 2020
@xiphmont @barrbrain
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
2c8cdcf 
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.
barrbrain pushed a commit to barrbrain/rav1e that referenced this issue May 28, 2020
@xiphmont @barrbrain
Fix for xiph#2212; Crash when using 4 tiles for 1080p 4:2:2 input
c8a9333 
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.
barrbrain pushed a commit that referenced this issue May 28, 2020
@xiphmont @barrbrain
Fix for #2212; Crash when using 4 tiles for 1080p 4:2:2 input
f37c13c 
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.
@xiphmont xiphmont mentioned this issue Jun 5, 2020
Merged
@vibhoothi vibhoothi mentioned this issue Sep 15, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xiphmont xiphmont

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@barrbrain @negge @tdaede @xiphmont @gnafuthegreat