Commit e7521d26 authored by Romain Vimont's avatar Romain Vimont Committed by Thomas Daede
Browse files

Refactor restoration units

A restoration unit may contain several super-blocks, and may be
"stretched" on borders, even across tile boundaries:
<https://github.com/xiph/rav1e/issues/631#issuecomment-454419152>

In the bitstream, it must be coded only for its first super-block, in
plane order. To do so, a "coded" flag was set the first time, so that
further super-blocks using the same restoration will not "code" it.

But this assumed that all super-blocks associated to a restoration unit
were encoded sequentially in plane order. With parallel tile encoding,
even with proper synchronization (preventing data races), this
introduces a race condition: a "stretched" restoration unit may not be
coded in its first super-block, corrupting the bitstream.

To avoid the problem, expose the restoration unit only for its first
super-block, by returning a Option<&(mut) RestorationUnit>. This also
avoids the need for any synchronization (a restoration unit will never
be retrieved by more than 1 tile).

At frame level, lrf_filter_frame() will still retrieve the correct
restoration unit for each super-block, by calling
restoration_unit_by_stripe().
parent b9fef7ce
......@@ -3217,15 +3217,9 @@ impl<'a> ContextWriter<'a> {
) {
if !fi.allow_intrabc { // TODO: also disallow if lossless
for pli in 0..PLANES {
let code;
let rp = &mut rs.planes[pli];
{
let ru = &mut rp.restoration_unit_as_mut(sbo);
code = !ru.coded;
ru.coded = true;
}
if code {
match rp.restoration_unit_as_mut(sbo).filter {
if let Some(filter) = rp.restoration_unit(sbo).map(|ru| ru.filter) {
match filter {
RestorationFilter::None => {
match rp.lrf_type {
RESTORE_WIENER => {
......
......@@ -778,14 +778,12 @@ fn wiener_stripe_filter<T: Pixel>(coeffs: [[i8; 3]; 2], fi: &FrameInvariants<T>,
#[derive(Copy, Clone, Debug)]
pub struct RestorationUnit {
pub filter: RestorationFilter,
pub coded: bool,
}
impl RestorationUnit {
pub fn default() -> RestorationUnit {
RestorationUnit {
filter: RestorationFilter::default(),
coded: false,
}
}
}
......@@ -830,11 +828,23 @@ impl RestorationPlane {
}
}
fn restoration_unit_index(&self, sbo: SuperBlockOffset) -> (usize, usize) {
(
(sbo.x >> self.sb_shift).min(self.cols - 1),
(sbo.y >> self.sb_shift).min(self.rows - 1),
)
fn restoration_unit_index(&self, sbo: SuperBlockOffset) -> Option<(usize, usize)> {
// there is 1 restoration unit for (1 << sb_shift) super-blocks
let mask = (1 << self.sb_shift) - 1;
let first_sbo = sbo.x & mask == 0 && sbo.y & mask == 0;
if first_sbo {
let x = sbo.x >> self.sb_shift;
let y = sbo.y >> self.sb_shift;
if x < self.cols && y < self.rows {
Some((x, y))
} else {
// this super-block will share the "stretched" restoration unit from its neighbours
None
}
} else {
// the restoration unit is ignored for others super-blocks
None
}
}
// Stripes are always 64 pixels high in a non-subsampled
......@@ -847,14 +857,17 @@ impl RestorationPlane {
)
}
pub fn restoration_unit(&self, sbo: SuperBlockOffset) -> &RestorationUnit {
let (x, y) = self.restoration_unit_index(sbo);
&self.units[y * self.cols + x]
pub fn restoration_unit(&self, sbo: SuperBlockOffset) -> Option<&RestorationUnit> {
self.restoration_unit_index(sbo).map(|(x, y)| &self.units[y * self.cols + x])
}
pub fn restoration_unit_as_mut(&mut self, sbo: SuperBlockOffset) -> &mut RestorationUnit {
let (x, y) = self.restoration_unit_index(sbo);
&mut self.units[y * self.cols + x]
pub fn restoration_unit_mut(&mut self, sbo: SuperBlockOffset) -> Option<&mut RestorationUnit> {
// cannot use map() due to lifetime constraints
if let Some((x, y)) = self.restoration_unit_index(sbo) {
Some(&mut self.units[y * self.cols + x])
} else {
None
}
}
pub fn restoration_unit_by_stripe(&self, stripenum: usize, rux: usize) -> &RestorationUnit {
......@@ -902,12 +915,12 @@ impl RestorationState {
}
}
pub fn restoration_unit(&self, sbo: SuperBlockOffset, pli: usize) -> &RestorationUnit {
self.planes[pli].restoration_unit(sbo)
}
pub fn has_restoration_unit(&self, sbo: SuperBlockOffset) -> bool {
let is_some = self.planes[0].restoration_unit(sbo).is_some();
debug_assert_eq!(is_some, self.planes[1].restoration_unit(sbo).is_some());
debug_assert_eq!(is_some, self.planes[2].restoration_unit(sbo).is_some());
pub fn restoration_unit_as_mut(&mut self, sbo: SuperBlockOffset, pli: usize) -> &mut RestorationUnit {
self.planes[pli].restoration_unit_as_mut(sbo)
is_some
}
pub fn lrf_filter_frame<T: Pixel>(&mut self, out: &mut Frame<T>, pre_cdef: &Frame<T>,
......
......@@ -1299,16 +1299,8 @@ pub fn rdo_loop_decision<T: Pixel>(sbo: SuperBlockOffset, fi: &FrameInvariants<T
}
}
let mut lrf_any_uncoded = false;
if fi.sequence.enable_restoration {
for pli in 0..PLANES {
let ru = fs.restoration.planes[pli].restoration_unit(sbo);
if !ru.coded {
lrf_any_uncoded = true;
break;
}
}
}
let lrf_any_uncoded =
fi.sequence.enable_restoration && fs.restoration.has_restoration_unit(sbo);
// CDEF/LRF decision iteration
// Start with a default of CDEF 0 and RestorationFilter::None
......@@ -1386,8 +1378,7 @@ pub fn rdo_loop_decision<T: Pixel>(sbo: SuperBlockOffset, fi: &FrameInvariants<T
// SgrProj LRF decision
for pli in 0..3 {
let ru = fs.restoration.planes[pli].restoration_unit(sbo);
if !ru.coded {
if fs.restoration.planes[pli].restoration_unit(sbo).is_some() {
for set in 0..16 {
let in_plane = &fs.input.planes[pli]; // reference
let ipo = sbo.plane_offset(&in_plane.cfg);
......@@ -1440,8 +1431,7 @@ pub fn rdo_loop_decision<T: Pixel>(sbo: SuperBlockOffset, fi: &FrameInvariants<T
if fi.sequence.enable_restoration {
for pli in 0..PLANES {
let ru = fs.restoration.planes[pli].restoration_unit_as_mut(sbo);
if !ru.coded {
if let Some(ru) = fs.restoration.planes[pli].restoration_unit_mut(sbo) {
ru.filter = best_lrf[pli];
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment