Add film grain support in VA-API #61
Conversation
This member was previously unset, set it during apply_sps().
Most accessors have already been removed, but some remain. Remove them.
We need to support new pools to cater to SVC content where multiple spatial layers are being decoded, otherwise we would have to clear bigger frames before decoding smaller layers to avoid leaving trash in them and breaking the decode process. This patch sets the stage by allowing multiple pools to exists, but only a single pool is created in the VA code for now. Subsequent patches will build on this one to actually allocate multiple pools depending on the content being decoded.
The previous commit added the capability of drawing frames from multiple pools. This commits follows up by actually creating more than one pool when needed.
We now have multiple pools available. Draw from the right pool when decoding SVC streams.
If we have multiple pools, we should split min_num_frames among them
All tiles belonging to the same tile group must be submitted to the vaapi driver as an array. The Intel driver, in particular, expects one array of Slice Parameters per instance of Slice Data when submitting the data for an AV1 Tile Group. It will produce a corrupted frame if the slice parameters are submitted separately (i.e.: in their own VABuffers). This was discovered by Nicolas Dufresne.
Add a new handle for post-processing. This use-case requires a surface for non-filtered data (for the decode process), and one surface for the filtered data (for display). This was previously impossible, as we only had one handle. Thus add a new type of handle and unite them through an enum. A next commit will make use of this handle to implement film grain in the AV1 VA-API code.
dwlsalmeida
force-pushed
the
film-grain
branch
from
e21eb8d
to
6dc76f4
Compare
Use the new VAAPI handle to add film grain support in the VAAPI AV1 decoder. One surface contains the grain and is meant to be displayed, while the other one does not. We need the decode surface to not have any grain, otherwise this would break the decoding process. The number of surfaces is doubled to accomodate for this, but only when film grain is active.
dwlsalmeida
force-pushed
the
film-grain
branch
from
6dc76f4
to
6265f89
Compare
|
Note to self: all CLs but the last two ones have been merged. The remaining CLs will need a rebase due to many conflicts (thankfully they are not too big) but we may also want to reconsider the workflow as I'm not too keen about turning the VAAPI handle into an enum just for this special case. |
|
@Gnurou having an extra surface is also important for other contexts, like using the preprocessor, which is why GStreamer has the same design @ndufresne correct me if I am wrong |
|
Correct, filmgrain is a postprocessing that must not be applied to reference frames. Though VA itself have limited inline postprocessing capabilities (which would require this). Vulkan Video fixes this issue. Encoders on the other end, always needs an extra set of surfaces, even when no preprocessing is taking place. This is to store the reconstruction frames. |
|
Yeah I am not against the extra surface which is indeed a necessity, just wondering if we could avoid the enum on the handle. I need to look at this more closely, but the general principle is of course good (and needed for compliance anyway :)) |
This PR adds film grain support to the VA-API code. It does so by first adding a new type of handle, and then making use of that handle in the AV1 VA-API decoder.
With this PR, all our film grain tests go from crashing to failing on Intel Gen12. This is a limitation of their implementation, which is not spec compliant and will also fail when using GStreamer.
I hear it should be good on Intel 13th gen onwards, but I do not have hardware to confirm. Meanwhile, users with unsupported hw (i.e. gen <=12) will get Intel's own implementation, which should be indistinguishable from the spec version for a human eye anyways.
This is built on top of #59, since it touches some bits that were changed in that PR as well, so it has the same dependencies as that PR.
@Gnurou Please test this on your end as well!
A late merry Christmas and a happy new year :)