Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

decoder/stateless: let the backends decide which resources they need #89

Merged
merged 22 commits into from
Jul 16, 2024

Conversation

Gnurou
Copy link
Collaborator

@Gnurou Gnurou commented Jul 12, 2024

This series fixes a design shortcoming in the decoder, namely that it was checking for output buffers availability before deciding to initiate a picture with the backend.

This was working well for the VAAPI backend, which must secure its output surface to create a VAPicture, but is not appropriate for V4L2 where the input and output queues are completely independent.

Each codec has a series of CLs culminating with let the backend decide which resources it needs, which moves the burden of checking output surfaces availability from the decoder to the VAAPI backend. That way the V4L2 backend is not constrained by output resources, and can decide itself which resources it needs to acquire to create a picture (in its case, an input buffer).

The other CLs include changes that are necessary to reach this state, e.g. rework of the backend interface to separate picture creation from submission when needed, and reordering of the format negotiation flow.

Sorry for the number of CLs ; however they should be bite-sized and relatively easy to understand.

new_picture is only used to allocate the resources needed for a new
frame and doesn't need this argument.
The decoder code was checking explicitly for output buffers
availability before decoding frames, but that requirement is not needed
by all backends - for example V4L2 stateless has different requirements.

Move this check to the backend by using the new_picture() method to try
and allocate all required resources for decoding a frame.
@Gnurou Gnurou requested a review from bgrzesik July 12, 2024 04:29
@Gnurou Gnurou self-assigned this Jul 12, 2024
@Gnurou Gnurou changed the title stateless/decoder: let the backends decide which resources they need decoder/stateless: let the backends decide which resources they need Jul 12, 2024
Copy link
Collaborator

@bgrzesik bgrzesik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left one comment with a "mayor nit" 😅 Otherwise LGTM 😄

src/decoder/stateless/h264.rs Outdated Show resolved Hide resolved
src/decoder/stateless/av1.rs Show resolved Hide resolved
Gnurou added 19 commits July 13, 2024 12:51
The decoder code was checking explicitly for output buffers
availability before decoding frames, but that requirement is not needed
by all backends - for example V4L2 stateless has different requirements.

Move this check to the backend by introducing a new_picture() method
that is responsible for allocating the resources required for a picture
and returning the expected error if it cannot secure them.
handle_frame cannot change the decoding state, so check for it before
processing all frames.
…are decoding

There is no reason to not pursue if we are not in a decoding state, so
move that check to a more appropriate place.
This will allow us to allocate all the frames required prior to
processing a superframe and make sure the whole superframe can be
processed in one go.
…ethod

This is a different procedure from decoding an regular frame, which
justifies having both separate. This will also let us pass the picture
to decode into to `handle_frame` without having to worry whether it will
be written or not.
…rames

If a frame has its show_existing_frame flag set, then it does not
require an output resource.
The decoder code was checking explicitly for output buffers
availability before decoding frames, but that requirement is not needed
by all backends - for example V4L2 stateless has different requirements.

Move this check to the backend by using the new_picture() method to try
and allocate all required pictures of a superframe prior to decoding it.
This argument is not needed when creating a new resource.
This can be set directly from the slice header's information.
…es availability

We don't need any resource to perform negotiation, so do that check
first.
The only time we access this member is shortly after updating it with
update_current_set_ids() from the slice's parameter. So let's just use
the slice data instead of adding state that may become invalid.
This is some frame-local state that we would eventually like to get rid
of, so don't use it when we can get the SPS ID through other means.
The decoder code was checking explicitly for output buffers availability
before decoding frames, but that requirement is not needed by all
backends - for example V4L2 stateless has different requirements.

Move this check to the backend by using the new_picture() method to try
and allocate a picture before processing it, and return the appropriate
error if none is available.
decode() was trying to process a whole temporal unit, which complicates
the code quite a bit: the decoder must notably check for available
resources for all the frames in the OBU, which requires to parse the
input several times.

Our contract is that the caller must check how much input has been
consumed and re-submit the remainder if it has not been entirely
processed. Leverage this and only process one OBU per call to decode(),
which simplifies the resource availability check and will also allow us
to move it into the backend.
…tion

We want to create pictures early so the backend can check whether all
required resources are available before processing. This requires
splitting new_picture into a method that allocates the picture and its
resources, and another one that initializes the picture parameters, so
they can be called from two different sites.
The decoder code was checking explicitly for output buffers availability
before decoding frames, but that requirement is not needed by all
backends - for example V4L2 stateless has different requirements.

Move this check to the backend by using the new_picture() method to try
and allocate a picture before processing it, and return the appropriate
error if none is available.
Copy link
Collaborator

@bgrzesik bgrzesik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

src/decoder/stateless/av1.rs Show resolved Hide resolved
src/decoder/stateless/h264.rs Outdated Show resolved Hide resolved
Failures in new_picture are usually due to temporary unavailability of
required resources and are recoverable. Introduce a new error type to
better handle this, and try to be more precise about the type of
resource missing so the client can handle each case properly.
@Gnurou Gnurou merged commit 6c56dab into main Jul 16, 2024
2 checks passed
@Gnurou Gnurou deleted the new_picture branch July 16, 2024 03:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants