refactor(client): ♻️ Move decoder out of ClientCoreContext; fix IDR race condition #2421
Conversation
zmerp
requested review from
shinyquagsire23,
ShootingKing-AM,
The-personified-devil and
Vixea
zmerp
changed the title
zmerp
force-pushed
the
decoder-refactor
branch
2 times, most recently
from
d4cda2c to
2c9442b
Compare
alvr/client_openxr/src/stream.rs
Outdated
| if let Some(source) = self.decoder_source.as_mut() { | ||
| while frame_result.is_none() && Instant::now() < frame_poll_deadline { | ||
| frame_result = source.get_frame(); | ||
| thread::yield_now(); |
There was a problem hiding this comment.
Does the image queue lock actually cause waiting or is this just a busy loop?
There was a problem hiding this comment.
This is actually a busy loop. When there is a healthy amount of buffering, this will succeed immediately without busy looping. The code is the same as before just moved from lib.rs to stream.rs.
There was a problem hiding this comment.
Yea ik, but I think we should still add like a tiny wait if we're at it
There was a problem hiding this comment.
Something like 200 microsecs or 500 microsecs
There was a problem hiding this comment.
By reading online, seems that yield() is generally discouraged, mainly because the implementation is platform dependent and even within the same platform is quite unpredictable. The thread could actually resume immediately or wait up to 3ms. So it should never be used unless we did extensive profiling checking it does exactly what we want. It's much better to use our own sleep to balance out CPU usage and latency depending on our needs. So i will change this.
|
Lgtm so far, but I also can't properly review due to complexity |
zmerp
force-pushed
the
decoder-refactor
branch
from
f9a045d to
c13df15
Compare
zmerp
force-pushed
the
decoder-refactor
branch
from
c13df15 to
17ada22
Compare
|
Tested and working |
This PR creates a new API for using the decoder. Removed the distinction between internal and external decoder, there is only an "external" implementation. The C API had been tweaked to reflect the internal changes. A significant improvement of this refactor is zero-copy decoder integration for visionOS.
This PR also fixed one bug where the video stream might start in a corrupted state because of a race condition.