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

Android: When the hardware/default decoder fails to initialize, fall back to software. #1933

Merged
2 changes: 1 addition & 1 deletion alvr/client_core/src/connection.rs
Expand Up @@ -457,7 +457,7 @@ fn connection_pipeline(

match maybe_packet {
Ok(ServerControlPacket::InitializeDecoder(config)) => {
decoder::create_decoder(config);
decoder::create_decoder(config, settings.video.force_software_decoder);
}
Ok(ServerControlPacket::Restarting) => {
info!("{SERVER_RESTART_MESSAGE}");
Expand Down
5 changes: 4 additions & 1 deletion alvr/client_core/src/decoder.rs
Expand Up @@ -7,6 +7,7 @@ use std::time::Duration;
#[derive(Clone)]
pub struct DecoderInitConfig {
pub codec: CodecType,
pub force_software_decoder: bool,
zarik5 marked this conversation as resolved.
Show resolved Hide resolved
pub max_buffering_frames: f32,
pub buffering_history_weight: f32,
pub options: Vec<(String, MediacodecDataType)>,
Expand All @@ -15,6 +16,7 @@ pub struct DecoderInitConfig {
pub static DECODER_INIT_CONFIG: Lazy<Mutex<DecoderInitConfig>> = Lazy::new(|| {
Mutex::new(DecoderInitConfig {
codec: CodecType::H264,
force_software_decoder: false,
max_buffering_frames: 1.0,
buffering_history_weight: 0.9,
options: vec![],
Expand All @@ -29,9 +31,10 @@ pub static DECODER_SOURCE: alvr_common::OptLazy<crate::platform::VideoDecoderSou

pub static EXTERNAL_DECODER: RelaxedAtomic = RelaxedAtomic::new(false);

pub fn create_decoder(lazy_config: DecoderInitializationConfig) {
pub fn create_decoder(lazy_config: DecoderInitializationConfig, force_software_decoder: bool) {
let mut config = DECODER_INIT_CONFIG.lock();
config.codec = lazy_config.codec;
config.force_software_decoder = force_software_decoder;

if EXTERNAL_DECODER.value() {
EVENT_QUEUE
Expand Down
166 changes: 107 additions & 59 deletions alvr/client_core/src/platform/android/decoder.rs
@@ -1,9 +1,9 @@
use crate::decoder::DecoderInitConfig;
use alvr_common::{
anyhow::{bail, Result},
anyhow::{anyhow, bail, Context, Result},
error, info,
parking_lot::{Condvar, Mutex},
warn, RelaxedAtomic,
show_err, warn, RelaxedAtomic,
};
use alvr_session::{CodecType, MediacodecDataType};
use ndk::{
Expand Down Expand Up @@ -144,6 +144,43 @@ impl Drop for VideoDecoderSource {
}
}

fn mime_for_codec(codec: CodecType) -> &'static str {
match codec {
CodecType::H264 => "video/avc",
CodecType::Hevc => "video/hevc",
}
}

// Attempts to create a MediaCodec, and then configure and start it.
fn decoder_attempt_setup(
codec_type: CodecType,
is_software: bool,
format: &MediaFormat,
image_reader: &ImageReader,
) -> Result<MediaCodec> {
let decoder = if is_software {
let sw_codec_name = match codec_type {
CodecType::H264 => "OMX.google.h264.decoder",
CodecType::Hevc => "OMX.google.hevc.decoder",
};
MediaCodec::from_codec_name(&sw_codec_name)
.ok_or(anyhow!("no such codec: {}", &sw_codec_name))?
} else {
let mime = mime_for_codec(codec_type);
MediaCodec::from_decoder_type(&mime)
.ok_or(anyhow!("unable to find decoder for mime type: {}", &mime))?
};
decoder
.configure(
&format,
Some(&image_reader.window()?),
MediaCodecDirection::Decoder,
)
.with_context(|| "failed to configure decoder")?;
decoder.start().with_context(|| "failed to start decoder")?;
Ok(decoder)
}

// Create a sink/source pair
pub fn video_decoder_split(
config: DecoderInitConfig,
Expand All @@ -167,26 +204,6 @@ pub fn video_decoder_split(
// 2x: keep the target buffering in the middle of the max amount of queuable frames
let available_buffering_frames = (2. * config.max_buffering_frames).ceil() as usize;

let mime = match config.codec {
CodecType::H264 => "video/avc",
CodecType::Hevc => "video/hevc",
};

let format = MediaFormat::new();
format.set_str("mime", mime);
format.set_i32("width", 512);
format.set_i32("height", 1024);
format.set_buffer("csd-0", &csd_0);

for (key, value) in &config.options {
match value {
MediacodecDataType::Float(value) => format.set_f32(key, *value),
MediacodecDataType::Int32(value) => format.set_i32(key, *value),
MediacodecDataType::Int64(value) => format.set_i64(key, *value),
MediacodecDataType::String(value) => format.set_str(key, value),
}
}

let mut image_reader = ImageReader::new_with_usage(
1,
1,
Expand Down Expand Up @@ -242,55 +259,86 @@ pub fn video_decoder_split(
.set_buffer_removed_listener(Box::new(|_, _| ()))
.unwrap();

let decoder = Arc::new(FakeThreadSafe(
MediaCodec::from_decoder_type(&mime).unwrap(),
));
let mime = mime_for_codec(config.codec);

let format = MediaFormat::new();
format.set_str("mime", mime);
// Given https://github.com/alvr-org/ALVR/pull/1933#discussion_r1431902906 - change at own risk.
// It might be harmless, it might not be, but it's definitely a risk.
format.set_i32("width", 512);
format.set_i32("height", 1024);
format.set_buffer("csd-0", &csd_0);

for (key, value) in &config.options {
match value {
MediacodecDataType::Float(value) => format.set_f32(key, *value),
MediacodecDataType::Int32(value) => format.set_i32(key, *value),
MediacodecDataType::Int64(value) => format.set_i64(key, *value),
MediacodecDataType::String(value) => format.set_str(key, value),
}
}

info!("Using AMediaCoded format:{} ", format);
decoder
.configure(
&format,
Some(&image_reader.window().unwrap()),
MediaCodecDirection::Decoder,
)
.unwrap();
decoder.start().unwrap();

{
let mut decoder_lock = decoder_sink.lock();
let preparing_decoder = if config.force_software_decoder {
decoder_attempt_setup(config.codec, true, &format, &image_reader)
} else {
// Hardware decoders sometimes fail at the CSD-0.
// May as well fall back if this occurs.
match decoder_attempt_setup(config.codec, false, &format, &image_reader) {
Ok(d) => Ok(d),
Err(e) => {
// would be "warn!" but this is a severe caveat and a pretty major error.
error!(
"Attempting software fallback due to error in default decoder: {:#}",
e
);
decoder_attempt_setup(config.codec, true, &format, &image_reader)
}
}
};

if let Ok(prepared_decoder) = preparing_decoder {
let decoder = Arc::new(FakeThreadSafe(prepared_decoder));

*decoder_lock = Some(Arc::clone(&decoder));
{
let mut decoder_lock = decoder_sink.lock();

decoder_ready_notifier.notify_one();
}
*decoder_lock = Some(Arc::clone(&decoder));

while running.value() {
match decoder.dequeue_output_buffer(Duration::from_millis(1)) {
Ok(DequeuedOutputBufferInfoResult::Buffer(buffer)) => {
// The buffer timestamp is actually nanoseconds
let presentation_time_ns = buffer.info().presentation_time_us();
decoder_ready_notifier.notify_one();
}

if let Err(e) =
decoder.release_output_buffer_at_time(buffer, presentation_time_ns)
{
error!("Decoder dequeue error: {e}");
while running.value() {
match decoder.dequeue_output_buffer(Duration::from_millis(1)) {
Ok(DequeuedOutputBufferInfoResult::Buffer(buffer)) => {
// The buffer timestamp is actually nanoseconds
let presentation_time_ns = buffer.info().presentation_time_us();

if let Err(e) =
decoder.release_output_buffer_at_time(buffer, presentation_time_ns)
{
error!("Decoder dequeue error: {e}");
}
}
}
Ok(DequeuedOutputBufferInfoResult::TryAgainLater) => thread::yield_now(),
Ok(i) => info!("Decoder dequeue event: {i:?}"),
Err(e) => {
error!("Decoder dequeue error: {e}");
Ok(DequeuedOutputBufferInfoResult::TryAgainLater) => thread::yield_now(),
Ok(i) => info!("Decoder dequeue event: {i:?}"),
Err(e) => {
error!("Decoder dequeue error: {e}");

// lessen logcat flood (just in case)
thread::sleep(Duration::from_millis(50));
// lessen logcat flood (just in case)
thread::sleep(Duration::from_millis(50));
}
}
}
}

// Destroy all resources
decoder_sink.lock().take(); // Make sure the shared ref is deleted first
decoder.stop().unwrap();
drop(decoder);
// Destroy all resources
decoder_sink.lock().take(); // Make sure the shared ref is deleted first
decoder.stop().unwrap();
drop(decoder);
} else {
show_err(preparing_decoder);
Copy link
Member

Choose a reason for hiding this comment

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

Don't try to print the whole object. Rather, if you handle both Ok and Err cases, prefer using match instead of if-let, then print only the error in the Err branch.

}

image_queue.lock().clear();
error!("FIXME: Leaking Imagereader!");
Expand Down
5 changes: 3 additions & 2 deletions alvr/common/src/logging.rs
Expand Up @@ -128,11 +128,12 @@ pub fn show_e_blocking<E: Display>(e: E) {
}

pub fn show_err<T, E: Display>(res: Result<T, E>) -> Option<T> {
res.map_err(|e| show_e_block(e, false)).ok()
res.map_err(|e| show_e_block(format!("{:#}", e), false))
.ok()
}

pub fn show_err_blocking<T, E: Display>(res: Result<T, E>) -> Option<T> {
res.map_err(|e| show_e_block(e, true)).ok()
res.map_err(|e| show_e_block(format!("{:#}", e), true)).ok()
}
Copy link
Member

Choose a reason for hiding this comment

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

Why are you changing this code? I'm surprised it even compiles. {:#} is valid for E: Debug but the type bound is not specified. In practice, try not to depend on Debug, rather depend on Display.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, an explanation of what happened here:
While testing the "no unwrap" changes, I found that I wasn't getting the "inner error" (i.e. the ErrorUnknown from the media error), just the context. Now, to me, this is very bad (because it makes it harder to debug codec error reports from users/etc).
However, I couldn't get the error details out of it without either duplicating the inner error details in the context or going through this. I'll change things to duplicate the inner error details in the context.

Copy link
Member

Choose a reason for hiding this comment

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

Remember you can get help from anyhow to format the error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out that wasn't necessary. Not sure what I missed the first time; doesn't matter in any case.
d3645fe undoes this.


pub trait ToAny<T> {
Expand Down
6 changes: 6 additions & 0 deletions alvr/session/src/settings.rs
Expand Up @@ -472,6 +472,11 @@ pub struct VideoConfig {
#[schema(flag = "steamvr-restart")]
pub encoder_config: EncoderConfig,

#[schema(strings(
help = "Attempts to use a software decoder on the device. Slow, but may work around broken codecs."
))]
pub force_software_decoder: bool,

pub mediacodec_extra_options: Vec<(String, MediacodecDataType)>,

#[schema(flag = "steamvr-restart")]
Expand Down Expand Up @@ -1272,6 +1277,7 @@ pub fn session_settings_default() -> SettingsDefault {
vertical_offset_deg: 0.0,
},
},
force_software_decoder: false,
color_correction: SwitchDefault {
enabled: true,
content: ColorCorrectionConfigDefault {
Expand Down