Since the enum is used as an index to arrays, it unfortunately can't
be converted to an enum class, but at least we can make sure to use it
with the qualified enum name to make things a bit clearer.
Those previous constants were only set and used to select the first and
second transforms done by the Decoder class. By turning it into a
struct, we can make the code a bit more legible while keeping those
transform modes the same size as before or smaller.
Note that some of the previous segmentation feature settings must be
preserved when a frame is decoded that doesn't use segmentation.
This change also allowed a few functions in Decoder to be made static.
Previously, we were using size_t, often coerced from bool or u8, to
index reference pairs. Now, they must either be taken directly from
named fields or indexed using the `ReferenceIndex` enum with options
`primary` and `secondary`. With a more explicit method of indexing
these, the compiler can aid in using reference pairs correctly, and
fuzzers may be able to detect undefined behavior more easily.
All state that needed to persist between calls to decode_block was
previously stored in plain Vector fields. This moves them into a struct
which sets a more explicit lifetime on that data. It may be possible to
store this data on the stack of a function with the appropriate
lifetime now that it is split into its own struct.
The default intra prediction mode was only used to set the sub-block
modes and the y prediction mode. Instead of storing it in a field, with
the sub modes are stored in an Array, we can just pull the last element
to set the y mode.
With the addition of this struct, both the bool to determine if coefs
should be parsed and the token parse itself can take specific
parameters.
This is the last step in parameterizing all the tree parsing, so the
old functions in TreeParser are now unused. This patch is very
satisfying :^)
There's still more work to be done to clean up how the parameters are
passed from Parser, but that's work for another day.
This adds a tree-parsing function that can be called statically from
specific trees' implementations in TreeParser, of which Partition is
the first. This way, all calls to tree parses will take the context
they need to be able to select a tree and probabilities, which will
allow removal of the state dependence in TreeParser on fields from
itself and Parser.
The two different mode sets are stored in single fields, and the
underlying values didn't overlap, so there was no reason to keep them
separate.
The enum is now an enum class as well, to enforce that almost all uses
of the enum are named. The only case where underlying values are used
is in lookup tables, but it may be worth abstracting that as well to
make array bounds more clear.
Integer overflow could sometimes occur due to counts going above 255,
where the values should instead be clamped at their maximum to avoid
wrapping to 0.
The above interpolation filter mode was being taken from the left side
instead, causing some parsing errors.
This also changes the magic number 3 to SWITCHABLE_FILTERS.
Unfortunately, the spec uses the magic number, so this value was taken
instead from the reference codec, libvpx.
This gets the decoder closer to fully parsing the second frame without
any errors. It will still be unable to output an inter-predicted frame.
The lack of output causes VideoPlayer to crash if it attempts to read
the buffers for frame 1, so it is still limited to the first frame.
The first keyframe of the test video can be decoded with these changes.
Raw memory allocations in the Parser have been replaced with Vector or
Array to avoid memory leaks and OOBs.
With this patch we are finally done with section 6.4.X of the spec :^)
The only parsing left to be done is 6.5.X, motion vector prediction.
Additionally, this patch fixes how MVs were being stored in the parser.
Originally, due to the spec naming two very different values very
similarly, these properties had totally wrong data types, but this has
now been rectified.
Though technically block decoding calls into some other incomplete
methods, so it isn't functionally complete yet. However, we are
very close to being done with the 6.4.X sections :)
These elements were being used in the new tokens implementation, so
support for them in the TreeParser has been added.
Additionally, this uncovered a bug where the nonzero contexts were
being cleared with the wrong size.
The class that was previously named Decoder handled section 6.X.X of
the spec, which actually deals with parsing out the syntax of the data,
not the actual decoding logic which is specified in section 8.X.X.
The new Decoder class will be in charge of owning and running the
Parser, as well as implementing all of the decoding behavior.
Additionally, this uncovered a couple bugs with existing code,
so those have been fixed. Currently, parsing a whole video does
fail because we are now using a new calculation for frame width,
but it hasn't been fully implemented yet.
Now TreeParser has mostly complete probability calculation
implementations for all currently used syntax elements. Some of these
calculation methods aren't actually finished because they use data
we have yet to parse in the Decoder, but they're close to finished.
With the progress made in the Decoder thus far, we have the ability
to support most of the syntax element counters in the tree parser.
Additionally, it will now crash when trying to count unsupported
elements.
This patch brings all of LibVideo up to the east-const style in the
project. Additionally, it applies a few fixes from the reviews in #8170
that referred to older LibVideo code.
The TreeParser requires information about a lot of the decoder's
current state in order to parse syntax tree elements correctly, so
there has to be some communication between the Decoder and the
TreeParser. Previously, the Decoder would copy its state to the
TreeParser when it changed, however, this was a poor choice. Now,
the TreeParser simply has a reference to its owning Decoder, and
accesses its state directly.
