Skip to content

Commit

Permalink
Fix for xiph#1601, xiph#1611 and xiph#1629 and possible xiph#1608
Browse files Browse the repository at this point in the history 
This patch set attempts to both improve cooperation between LRF and
tiling setup, but also eliminate several illegal configurations.

The patch strategy is to consistently eliminate evaluation of
'stretch' superblocks in RDO of LRUs for the loop restoration filter.
This primarily affects the case where a tile consists of
primarily/exclusively SBs that belong to an LRU stretched from some
other tile.
  • Loading branch information
@xiphmont
xiphmont committed Sep 5, 2019
1 parent cc170c2 commit 2bdc019
Show file tree
Hide file tree
Showing 7 changed files with 227 additions and 149 deletions.
3 changes: 2 additions & 1 deletion src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3603,7 +3603,8 @@ impl<'a> ContextWriter<'a> {
if !fi.allow_intrabc {
// TODO: also disallow if lossless
let rp = &mut rs.planes[pli];
if let Some(filter) = rp.restoration_unit(sbo).map(|ru| ru.filter) {
if let Some(filter) = rp.restoration_unit(sbo, true).map(|ru| ru.filter)
{
match filter {
RestorationFilter::None => match rp.rp_cfg.lrf_type {
RESTORE_WIENER => {
Expand Down
40 changes: 30 additions & 10 deletions src/encoder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3238,13 +3238,23 @@ fn encode_tile<'a, T: Pixel>(
// queue our superblock for when the LRU is complete
sbs_qe.cdef_coded = cw.bc.cdef_coded;
for pli in 0..PLANES {
let lru_index = ts.restoration.planes[pli]
.restoration_unit_countable(tile_sbo)
as i32;
sbs_qe.lru_index[pli] = lru_index;
if ts.restoration.planes[pli].restoration_unit_last_sb(fi, tile_sbo)
if let Some((lru_x, lru_y)) =
ts.restoration.planes[pli].restoration_unit_index(tile_sbo, false)
{
last_lru_ready[pli] = lru_index;
let lru_index = ts.restoration.planes[pli]
.restoration_unit_countable(lru_x, lru_y)
as i32;
sbs_qe.lru_index[pli] = lru_index;
if ts.restoration.planes[pli]
.restoration_unit_last_sb_for_rdo(fi, tile_sbo)
{
last_lru_ready[pli] = lru_index;
check_queue = true;
}
} else {
// we're likely in an area stretched into a new tile
// tag this SB to be ignored in LRU decisions
sbs_qe.lru_index[pli] = -1;
check_queue = true;
}
}
Expand All @@ -3262,7 +3272,7 @@ fn encode_tile<'a, T: Pixel>(
if check_queue {
// yes, this entry is ready
if qe.cdef_coded || fi.sequence.enable_restoration {
// only do this once for a given LRU.
// only RDO once for a given LRU.

// One quirk worth noting: LRUs in different planes
// may be different sizes; eg, one chroma LRU may
Expand All @@ -3274,17 +3284,25 @@ fn encode_tile<'a, T: Pixel>(
// RDO happens on all LRUs within the confines of the
// biggest, all together. If any of this SB's planes'
// LRUs are RDOed, in actuality they all are.

// SBs tagged with a lru index of -1 are ignored in
// LRU coding/rdoing decisions (but still need to rdo
// for cdef).
let mut already_rdoed = false;
for pli in 0..PLANES {
if qe.lru_index[pli] <= last_lru_rdoed[pli] {
if qe.lru_index[pli] != -1
&& qe.lru_index[pli] <= last_lru_rdoed[pli]
{
already_rdoed = true;
break;
}
}
if !already_rdoed {
rdo_loop_decision(qe.sbo, fi, ts, &mut cw, &mut w);
for pli in 0..PLANES {
if last_lru_rdoed[pli] < qe.lru_index[pli] {
if qe.lru_index[pli] != -1
&& last_lru_rdoed[pli] < qe.lru_index[pli]
{
last_lru_rdoed[pli] = qe.lru_index[pli];
}
}
Expand All @@ -3293,7 +3311,9 @@ fn encode_tile<'a, T: Pixel>(
// write LRF information
if fi.sequence.enable_restoration {
for pli in 0..PLANES {
if last_lru_coded[pli] < qe.lru_index[pli] {
if qe.lru_index[pli] != -1
&& last_lru_coded[pli] < qe.lru_index[pli]
{
last_lru_coded[pli] = qe.lru_index[pli];
cw.write_lrf(&mut w, fi, &mut ts.restoration, qe.sbo, pli);
}
Expand Down
42 changes: 36 additions & 6 deletions src/lrf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use crate::frame::PlaneSlice;
use crate::lrf_simd::*;
use crate::util::clamp;
use crate::util::CastFromPrimitive;
use crate::util::ILog;
use crate::util::Pixel;

use std::cmp;
Expand Down Expand Up @@ -1248,6 +1249,8 @@ impl RestorationState {
let PlaneConfig { xdec, ydec, .. } = input.planes[1].cfg;
// stripe size is decimated in 4:2:0 (and only 4:2:0)
let stripe_uv_decimate = if xdec > 0 && ydec > 0 { 1 } else { 0 };
let y_sb_log2 = if fi.sequence.use_128x128_superblock { 7 } else { 6 };
let uv_sb_log2 = y_sb_log2 - stripe_uv_decimate;

let (lrf_y_shift, lrf_uv_shift) = if fi.sequence.enable_large_lru {
// Largest possible restoration unit size (256) for both luma and chroma
Expand All @@ -1258,13 +1261,40 @@ impl RestorationState {
(lrf_y_shift, lrf_y_shift + stripe_uv_decimate)
};

let mut y_unit_size = 1 << RESTORATION_TILESIZE_MAX_LOG2 - lrf_y_shift;
let mut uv_unit_size = 1 << RESTORATION_TILESIZE_MAX_LOG2 - lrf_uv_shift;

// Right now we defer to tiling setup: don't choose an LRU size
// large enough that a tile is not an integer number of LRUs
// wide/high.
if fi.tiling.cols > 1 || fi.tiling.rows > 1 {
let tile_y_unit_width = fi.tiling.tile_width_sb << y_sb_log2;
let tile_y_unit_height = fi.tiling.tile_height_sb << y_sb_log2;
let tile_uv_unit_width = fi.tiling.tile_width_sb << uv_sb_log2;
let tile_uv_unit_height = fi.tiling.tile_height_sb << uv_sb_log2;

y_unit_size = y_unit_size.min(tile_y_unit_width).min(tile_y_unit_height);
uv_unit_size =
uv_unit_size.min(tile_uv_unit_width).min(tile_uv_unit_height);

// But it's actually worse: LRUs can't span tiles (in our design
// that is, spec allows it). However, the spec mandates the last
// LRU stretches forward into any less-than-half-LRU span of
// superblocks at the right and bottom of a frame. These
// superblocks may well be in a different tile! Even if LRUs are
// minimum size (one superblock), when the right or bottom edge of
// the frame is a superblock that's less than half the
// width/height of a normal superblock, the LRU is forced by the
// spec to span into it (and thus a different tile). Tiling is
// under no such restriction; it could decide the right/left
// sliver will be in its own tile row/column. We can't disallow
// the combination here. The tiling code will have to either
// prevent it or tolerate it. (prayer mechanic == Issue #1629).
}

// derive the rest
let y_unit_log2 = RESTORATION_TILESIZE_MAX_LOG2 - lrf_y_shift;
let uv_unit_log2 = RESTORATION_TILESIZE_MAX_LOG2 - lrf_uv_shift;
let y_unit_size = 1 << y_unit_log2;
let uv_unit_size = 1 << uv_unit_log2;
let y_sb_log2 = if fi.sequence.use_128x128_superblock { 7 } else { 6 };
let uv_sb_log2 = y_sb_log2 - stripe_uv_decimate;
let y_unit_log2 = y_unit_size.ilog() - 1;
let uv_unit_log2 = uv_unit_size.ilog() - 1;
let y_cols = ((fi.width + (y_unit_size >> 1)) / y_unit_size).max(1);
let y_rows = ((fi.height + (y_unit_size >> 1)) / y_unit_size).max(1);
let uv_cols = ((((fi.width + (1 << xdec >> 1)) >> xdec)
Expand Down
220 changes: 118 additions & 102 deletions src/rdo.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1771,113 +1771,129 @@ pub fn rdo_loop_decision<T: Pixel>(
let height = (wh + (1 << ydec >> 1)) >> ydec;
// which LRU are we currently testing against?
let rp = &ts.restoration.planes[pli];
if let Some((tile_lru_x, tile_lru_y)) =
rp.restoration_unit_index(tile_sbo)
{
if let Some((loop_tile_lru_x, loop_tile_lru_y)) =
rp.restoration_unit_index(loop_tile_sbo)
{
let lru_x = loop_tile_lru_x - tile_lru_x;
let lru_y = loop_tile_lru_y - tile_lru_y;

match best_lrf[pli][lru_y][lru_x] {
RestorationFilter::None {} => {
err += rdo_loop_plane_error(
loop_sbo,
loop_tile_sbo,
1,
fi,
ts,
&cw.bc.blocks.as_const(),
&lrf_input,
if let (
Some((tile_lru_x, tile_lru_y)),
Some((loop_tile_lru_x, loop_tile_lru_y))
) = (
rp.restoration_unit_index(tile_sbo, false),
rp.restoration_unit_index(loop_tile_sbo, false)
) {
let lru_x = loop_tile_lru_x - tile_lru_x;
let lru_y = loop_tile_lru_y - tile_lru_y;

match best_lrf[pli][lru_y][lru_x] {
RestorationFilter::None {} => {
err += rdo_loop_plane_error(
loop_sbo,
loop_tile_sbo,
1,
fi,
ts,
&cw.bc.blocks.as_const(),
&lrf_input,
pli,
);

rate += if fi.sequence.enable_restoration {
cw.count_lrf_switchable(
w,
&ts.restoration.as_const(),
best_lrf[pli][lru_y][lru_x],
pli,
)
} else {
0 // no relative cost differeneces to different
// CDEF params. If cdef is on, it's a wash.
};
}
RestorationFilter::Sgrproj { set, xqd } => {
// only run on this superblock
// if height is 128x128, we'll need to run two stripes
let loop_po =
loop_sbo.plane_offset(&lrf_input.planes[pli].cfg);
if height > 64 {
let loop_po2 =
PlaneOffset { x: loop_po.x, y: loop_po.y + 64 };
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
64,
width,
64,
&lrf_input.planes[pli].slice(loop_po),
&lrf_input.planes[pli].slice(loop_po),
&mut lrf_output.planes[pli].mut_slice(loop_po),
);
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
64,
width,
64,
&lrf_input.planes[pli].slice(loop_po2),
&lrf_input.planes[pli].slice(loop_po2),
&mut lrf_output.planes[pli].mut_slice(loop_po2),
);
} else {
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
height,
width,
height,
&lrf_input.planes[pli].slice(loop_po),
&lrf_input.planes[pli].slice(loop_po),
&mut lrf_output.planes[pli].mut_slice(loop_po),
);

rate += if fi.sequence.enable_restoration {
cw.count_lrf_switchable(
w,
&ts.restoration.as_const(),
best_lrf[pli][lru_y][lru_x],
pli,
)
} else {
0 // no relative cost differeneces to different
// CDEF params. If cdef is on, it's a wash.
};
}
RestorationFilter::Sgrproj { set, xqd } => {
// only run on this superblock
// if height is 128x128, we'll need to run two stripes
let loop_po =
loop_sbo.plane_offset(&lrf_input.planes[pli].cfg);
if height > 64 {
let loop_po2 =
PlaneOffset { x: loop_po.x, y: loop_po.y + 64 };
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
64,
width,
64,
&lrf_input.planes[pli].slice(loop_po),
&lrf_input.planes[pli].slice(loop_po),
&mut lrf_output.planes[pli].mut_slice(loop_po),
);
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
64,
width,
64,
&lrf_input.planes[pli].slice(loop_po2),
&lrf_input.planes[pli].slice(loop_po2),
&mut lrf_output.planes[pli].mut_slice(loop_po2),
);
} else {
sgrproj_stripe_filter(
set,
xqd,
fi,
&mut ts.stripe_filter_buffer,
width,
height,
width,
height,
&lrf_input.planes[pli].slice(loop_po),
&lrf_input.planes[pli].slice(loop_po),
&mut lrf_output.planes[pli].mut_slice(loop_po),
);
}
}
RestorationFilter::Wiener { .. } => unreachable!(), // coming soon
}
let this_err = rdo_loop_plane_error(
loop_sbo,
loop_tile_sbo,
1,
fi,
ts,
&cw.bc.blocks.as_const(),
&lrf_output,
pli,
);
err += this_err;
let this_rate = cw.count_lrf_switchable(
w,
&ts.restoration.as_const(),
best_lrf[pli][lru_y][lru_x],
pli,
);
rate += this_rate;
RestorationFilter::Wiener { .. } => unreachable!(), // coming soon
}
let this_err = rdo_loop_plane_error(
loop_sbo,
loop_tile_sbo,
1,
fi,
ts,
&cw.bc.blocks.as_const(),
&lrf_output,
pli,
);
err += this_err;
let this_rate = cw.count_lrf_switchable(
w,
&ts.restoration.as_const(),
best_lrf[pli][lru_y][lru_x],
pli,
);
rate += this_rate;
} else {
// No LRU here, compute error directly from CDEF output.
err += rdo_loop_plane_error(
loop_sbo,
loop_tile_sbo,
1,
fi,
ts,
&cw.bc.blocks.as_const(),
&lrf_input,
pli,
);
// no aelative cost differeneces to different
// CDEF params. If cdef is on, it's a wash.
// rate += 0;
}
}

let cost = compute_rd_cost(fi, rate, err);
if best_cost < 0. || cost < best_cost {
best_cost = cost;
Expand Down Expand Up @@ -1945,7 +1961,7 @@ pub fn rdo_loop_decision<T: Pixel>(
y: tile_sbo.0.y + loop_sbo.0.y,
});
if fi.sequence.enable_restoration
&& ts.restoration.has_restoration_unit(loop_tile_sbo, pli)
&& ts.restoration.has_restoration_unit(loop_tile_sbo, pli, false)
{
let ref_plane = &ts.input.planes[pli]; // reference
let lrf_in_plane = &lrf_input.planes[pli];
Expand Down Expand Up @@ -1989,7 +2005,7 @@ pub fn rdo_loop_decision<T: Pixel>(
RestorationFilter::Sgrproj { set, xqd: [xqd0, xqd1] };
if let RestorationFilter::Sgrproj { set, xqd } = current_lrf {
// one stripe at a time
// doesn't consider stretch yet
// do not consider any stretch; we might fall off the tile
for y in (0..unit_height).step_by(stripe_height) {
let stripe_po =
PlaneOffset { x: loop_po.x, y: loop_po.y + y as isize };
Expand Down
Loading

0 comments on commit 2bdc019

Please sign in to comment.