-
Notifications
You must be signed in to change notification settings - Fork 12
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
decoders: Add stateful decoder API #62
base: main
Are you sure you want to change the base?
Conversation
Add an initial version of stateful decoder API.
@InternetOfTofu can you provide at least one implementation so that we know this works? Also, IIRC, someone from @Gnurou 's team was already working on something similar |
That someone is actually @InternetOfTofu :) Sorry for the delay, I'll try to review this this week! |
Other(#[from] anyhow::Error), | ||
} | ||
|
||
#[derive(Copy, Clone, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since ccdec has a similar enum, we should probably have a single declaration in lib.rs since we already have DecodedFormat
there.
|
||
#[derive(Copy, Clone, Debug)] | ||
|
||
pub enum ColorGamut { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now we don't have any way to manage color properties, even for stateless decoders. Let's drop this for now as it is not needed for basic decoding.
Smpte170m, | ||
} | ||
|
||
#[derive(Copy, Clone, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Iec61966_2_1, | ||
} | ||
|
||
#[derive(Copy, Clone, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Smpte170m, | ||
} | ||
|
||
#[derive(Copy, Clone, Debug)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
buffer: Option<Box<dyn AsBytes>>, | ||
} | ||
|
||
pub enum StatefulDecoderEvent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect you can reuse DecoderEvent
(and the DecodedHandle
trait) as-is for the stateful decoder. It will align the interfaces and make the code simpler somehow.
FrameReady(VideoFrame), | ||
/// The format of the stream has changed and action is required. A VideoFrame | ||
/// with no buffer is returned. | ||
FormatChanged(VideoFrame), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to return a videoframe here?
Closed, | ||
} | ||
|
||
pub struct StatefulDecoder<C, B> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first use of the stateful decoder interface should be as a stateful wrapper to any stateless decoder, i.e. an adapter that turns a stateless decoder into a stateful one. That adapter maybe be implemented as a StatefulDecoderBackend
, but for simplicity I'd recommend writing it as an implementation of StatefulDecoder
first and see what parts we can share with other stateful decoder implementations.
So I'd suggest starting with the design of this wrapper first to guide the interface. Starting from any stateless decoder, the stateful wrapper should
- Accept input buffers and queue them until they can be processed,
- Try to submit work to the stateless decoder, hold if the work cannot be submitted to retry later, and process the events of the stateless decoder, forwarding them to the caller if needed (e.g. decoded frames),
... and I think that's mostly it. The simple_playback_loop
should give a good example of how things should work, as its purpose is to provide some stateful-ish behavior to our stateless decoders. I will be replaced it by the stateful decoder once it is available.
Add an initial version of stateful decoder API.