They have different paths and ASM functions.

Assert that BlockOffset::with_offset() never wraps due to negative
offsets.

Boolean variables were used to check left and top boundaries, but the
BlockOffset were created with negative values regardless.

Refactor to never create negative block offsets.

Fixes <https://github.com/xiph/rav1e/issues/1103>.

The code was inspired by libaom, which calls scan_blk_mbmi()
unconditionally:
<https://aomedia.googlesource.com/aom/+/9a7e8768d39793963725da754e61d8115e2404d0/av1/common/mvref_common.c#490>
<https://aomedia.googlesource.com/aom/+/9a7e8768d39793963725da754e61d8115e2404d0/av1/common/mvref_common.c#549>

However, the boundaries are checked in the implementation of
scan_blk_mbmi():
<https://aomedia.googlesource.com/aom/+/9a7e8768d39793963725da754e61d8115e2404d0/av1/common/mvref_common.c#251>

Check that the given offset will not make the resulting block offsets
negative (and wrap).

Fixes <https://github.com/xiph/rav1e/issues/1103>.

They are redundant with those of FrameBlocks.

FrameBlocks is part of the BlockContext structure. Many functions which
used only the FrameBlocks (but not the other BlockContext fields)
received a reference to the whole BlockContext as parameter.

FrameBlocks will survive tiles-encoding: it will be accessed through a
tiled version for each tile, then frame-wise after all tiles are
encoded.

However, BlockContext will only be used tile-wise: there will be one
instance per tile, destroyed once the tile is encoded. Therefore,
frame-wise functions may not receive the whole BlockContext anymore
(all tile-specific BlockContext instances won't exist anymore).

TD-Linux on IRC:

> superframe anti emulation was dropped from the spec mid development

Now that Index and IndexMut are implemented with BlockOffset index for
FrameBlocks, remove at() and at_mut().

For convenience, implement Index and IndexMut traits so that we can
index a block directly by a BlockOffset instance.

Some FrameBlocks setters used the local for_each() method, but some
others reimplemented it manually. Make them also use for_each().

Some BlockContext methods are specific to FrameBlocks, so move them.

With tiling, the lifetime of FrameBlocks will exceed that of
BlockContext (which will exist only during tile encoding).

This paves the way to move FrameBlocks out of BlockContext.

BlockContext::get_mode() was never used. Remove it.

The methods above_of(), left_of() and above_left_of() returned the
matching block by value, or a default block if the offset resulted in a
block outside boundaries.

The Block structure is quite big (std::mem::size_of::<Block>() == 120).
For reading a field, it is probably not optimal to return a whole Block
copy or a new default block (although the compiler might optimize such
accesses).

Moreover, the boundaries checks were often redundant, because already
done by the callers.

Instead, let the callers check boundaries and return a reference to the
matching block.

Rewrite conditions to make them easier to read.

Moves all functions that previously used usize to this type.
Instead of direct conversions to a slot number, use a to_index fn.

This also changes the size of the global mv state and context
ref counting arrays as they don't need LAST_FRAME.

Yushin Cho authored 5 years ago and Yushin Cho committed 5 years ago

Enabled for speed <= 2, i.e. when
fi.config.speed_settings.rdo_tx_decision == true.

AWCY result for speed 2

   PSNR | PSNR Cb | PSNR Cr | PSNR HVS |    SSIM | MS SSIM | CIEDE 2000
-1.3858 | -1.1944 | -0.8549 |  -0.9902 | -1.3141 | -0.9488 |    -1.1913

With encoding time increased < ~5%.

Monty Montgomery authored 5 years ago and Yushin Cho committed 5 years ago

Subpartitioning the luma TX to sub-partition size broke the Chroma deblocking code.
This patch implements proper luma/chroma tx size determination in the deblocking code.

Yushin Cho authored 5 years ago and Yushin Cho committed 5 years ago

Also add asserts to check the # of tx block for chroma is only one.
Later, this asserts become not valid if partition size is > 64x64
and chroma format is not 420.

Yushin Cho authored 5 years ago and Yushin Cho committed 5 years ago

Requireed sub functions are:
- get_tx_size_context()
- tx_size_to_depth()
- bsize_to_max_depth()
- bsize_to_tx_size_cat()
and etc.

Also add sub_tx_size_map[] table.

Yushin Cho authored 5 years ago and Yushin Cho committed 5 years ago

Also add reset_left_tx_context() function to BlockContext.

BlockContext::checkpoint() returned a whole copy of BlockContext, while
only a subset of its fields were actually part of the checkpoint. The
other fields, unused, were filled by default values.

Instead, use a structure dedicated to BlockContext checkpoints,
containing only the relevant fields.

In BlockContext, the blocks were stored as Vec<Vec<Block>>. Use a
dedicated structure instead.

This will allow to provide a tiled view (TileBlocks).

BlockContext was borrowed but only read.

Tidy up the loop as well.

Thomas Daede authored 5 years ago and Luca Barbato committed 5 years ago

This shows a pretty nice improvement of ~15% on encode block
benchmarks.

Semantically, the functions want to receive either Some reference or
None. They don't care whether the actual object happens to be
atomically refcounted.

As a downside, the caller is required to do some conversions.

> Don't pass a smart pointer as a function parameter unless you want to
> use or manipulate the smart pointer itself, such as to share or
> transfer ownership.
                      (Herb Sutter)

<https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/>

This is redundant. For consistency, keep blk_w and blk_h.

An optional tempoary plane is passed from callers to callees to avoid
reallocations. However, the callers of diamond_me_search() need not
provide this plane; they just need a flag to enable subpixel motion
estimation.

The temporary plane need not be provided by the caller of
sub_pixel_me() and telescopic_subpel_search().

Expose it only when it allows to avoid reallocations between several
function calls.

In motion estimation, several functions received both the offset
expressed in blocks and in pixels for the luma plane. This information
is redundant: a block offset is trivially convertible to a luma plane
offset.

With tiling, we need to manage both absolute offsets (relative to the
frame) and offsets relative to the current tile. This will be more
simple without duplication.

Like previous commits did for BlockOffset and SuperBlockOffset.