From 70a61c2b383319cb988fee4feaef7bde1de98a14 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Sun, 17 Dec 2023 22:14:25 +0000 Subject: [PATCH 1/9] Android: When the hardware/default decoder fails to initialize, fall back to software. --- alvr/client_core/src/decoder.rs | 9 ++ .../src/platform/android/decoder.rs | 99 +++++++++++++------ 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/alvr/client_core/src/decoder.rs b/alvr/client_core/src/decoder.rs index 83068cb3a8..2291fc2479 100644 --- a/alvr/client_core/src/decoder.rs +++ b/alvr/client_core/src/decoder.rs @@ -7,6 +7,9 @@ use std::time::Duration; #[derive(Clone)] pub struct DecoderInitConfig { pub codec: CodecType, + pub width: i32, + pub height: i32, + pub force_software_decoder: bool, pub max_buffering_frames: f32, pub buffering_history_weight: f32, pub options: Vec<(String, MediacodecDataType)>, @@ -15,6 +18,12 @@ pub struct DecoderInitConfig { pub static DECODER_INIT_CONFIG: Lazy> = Lazy::new(|| { Mutex::new(DecoderInitConfig { codec: CodecType::H264, + // TODO: Actually get the real values for these and update accordingly. + // These are inherited from the previous code to avoid breaking anything. + // Would also be nice if force_software_decoder was configurable. + width: 512, + height: 1024, + force_software_decoder: false, max_buffering_frames: 1.0, buffering_history_weight: 0.9, options: vec![], diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index 110ea53d80..b6cd2632e9 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -1,6 +1,6 @@ use crate::decoder::DecoderInitConfig; use alvr_common::{ - anyhow::{bail, Result}, + anyhow::{bail, Result, anyhow}, error, info, parking_lot::{Condvar, Mutex}, warn, RelaxedAtomic, @@ -144,6 +144,30 @@ impl Drop for VideoDecoderSource { } } +// Configure & start a MediaCodec. +fn decoder_configure_and_start(decoder: MediaCodec, format: &MediaFormat, image_reader: &ImageReader) -> Result { + decoder + .configure( + &format, + Some(&image_reader.window().unwrap()), + MediaCodecDirection::Decoder, + )?; + decoder.start()?; + Ok(decoder) +} + +// Creates a hardware (or assumed hardware) MediaCodec and then configure and start it. +fn decoder_try_hardware(mime: &str, format: &MediaFormat, image_reader: &ImageReader) -> Result { + let tmp = MediaCodec::from_decoder_type(&mime).ok_or(anyhow!("unable to find decoder for mime type: {}", &mime))?; + decoder_configure_and_start(tmp, &format, &image_reader) +} + +// Creates a software MediaCodec and then configure and start it. +fn decoder_try_software(codec_name: &str, format: &MediaFormat, image_reader: &ImageReader) -> Result { + let tmp = MediaCodec::from_codec_name(&codec_name).ok_or(anyhow!("no such codec: {}", &codec_name))?; + decoder_configure_and_start(tmp, &format, &image_reader) +} + // Create a sink/source pair pub fn video_decoder_split( config: DecoderInitConfig, @@ -167,26 +191,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, @@ -242,19 +246,50 @@ 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 = match config.codec { + CodecType::H264 => "video/avc", + CodecType::Hevc => "video/hevc", + }; + + let sw_codec_name = match config.codec { + CodecType::H264 => "OMX.google.h264.decoder", + CodecType::Hevc => "OMX.google.hevc.decoder", + }; + + let format = MediaFormat::new(); + format.set_str("mime", mime); + format.set_i32("width", config.width); + format.set_i32("height", config.height); + 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 preparing_decoder = if config.force_software_decoder { + decoder_try_software(sw_codec_name, &format, &image_reader).unwrap() + } else { + // Hardware decoders sometimes fail at the CSD-0. + // May as well fall back if this occurs. + match decoder_try_hardware(mime, &format, &image_reader) { + Ok(d) => d, + Err(e) => { + error!("Attempting software fallback due to error in default decoder: {}", e); + decoder_try_software(sw_codec_name, &format, &image_reader).unwrap() + } + } + }; + + let decoder = Arc::new(FakeThreadSafe( + preparing_decoder, + )); { let mut decoder_lock = decoder_sink.lock(); From ab65b9407aac409ff9f8c357a86d6c1a180ee4de Mon Sep 17 00:00:00 2001 From: 20kdc Date: Sun, 17 Dec 2023 23:15:06 +0000 Subject: [PATCH 2/9] Forgot to "cargo fmt --all", thank you CI --- .../src/platform/android/decoder.rs | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index b6cd2632e9..3585357f5f 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -1,6 +1,6 @@ use crate::decoder::DecoderInitConfig; use alvr_common::{ - anyhow::{bail, Result, anyhow}, + anyhow::{anyhow, bail, Result}, error, info, parking_lot::{Condvar, Mutex}, warn, RelaxedAtomic, @@ -145,26 +145,39 @@ impl Drop for VideoDecoderSource { } // Configure & start a MediaCodec. -fn decoder_configure_and_start(decoder: MediaCodec, format: &MediaFormat, image_reader: &ImageReader) -> Result { - decoder - .configure( - &format, - Some(&image_reader.window().unwrap()), - MediaCodecDirection::Decoder, - )?; +fn decoder_configure_and_start( + decoder: MediaCodec, + format: &MediaFormat, + image_reader: &ImageReader, +) -> Result { + decoder.configure( + &format, + Some(&image_reader.window().unwrap()), + MediaCodecDirection::Decoder, + )?; decoder.start()?; Ok(decoder) } // Creates a hardware (or assumed hardware) MediaCodec and then configure and start it. -fn decoder_try_hardware(mime: &str, format: &MediaFormat, image_reader: &ImageReader) -> Result { - let tmp = MediaCodec::from_decoder_type(&mime).ok_or(anyhow!("unable to find decoder for mime type: {}", &mime))?; +fn decoder_try_hardware( + mime: &str, + format: &MediaFormat, + image_reader: &ImageReader, +) -> Result { + let tmp = MediaCodec::from_decoder_type(&mime) + .ok_or(anyhow!("unable to find decoder for mime type: {}", &mime))?; decoder_configure_and_start(tmp, &format, &image_reader) } // Creates a software MediaCodec and then configure and start it. -fn decoder_try_software(codec_name: &str, format: &MediaFormat, image_reader: &ImageReader) -> Result { - let tmp = MediaCodec::from_codec_name(&codec_name).ok_or(anyhow!("no such codec: {}", &codec_name))?; +fn decoder_try_software( + codec_name: &str, + format: &MediaFormat, + image_reader: &ImageReader, +) -> Result { + let tmp = MediaCodec::from_codec_name(&codec_name) + .ok_or(anyhow!("no such codec: {}", &codec_name))?; decoder_configure_and_start(tmp, &format, &image_reader) } @@ -281,15 +294,16 @@ pub fn video_decoder_split( match decoder_try_hardware(mime, &format, &image_reader) { Ok(d) => d, Err(e) => { - error!("Attempting software fallback due to error in default decoder: {}", e); + error!( + "Attempting software fallback due to error in default decoder: {}", + e + ); decoder_try_software(sw_codec_name, &format, &image_reader).unwrap() } } }; - let decoder = Arc::new(FakeThreadSafe( - preparing_decoder, - )); + let decoder = Arc::new(FakeThreadSafe(preparing_decoder)); { let mut decoder_lock = decoder_sink.lock(); From f8fdf7fb49fb67d6d7cb6e4a8cc363814bf6ba44 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Mon, 18 Dec 2023 20:47:24 +0000 Subject: [PATCH 3/9] Android: Reduce unwraps in decoder setup to one unified place so that can be figured out later --- .../src/platform/android/decoder.rs | 72 +++++++++---------- 1 file changed, 32 insertions(+), 40 deletions(-) diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index 3585357f5f..392532954e 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -144,43 +144,43 @@ impl Drop for VideoDecoderSource { } } -// Configure & start a MediaCodec. -fn decoder_configure_and_start( - decoder: MediaCodec, +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 { + 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().unwrap()), + Some(&image_reader.window()?), MediaCodecDirection::Decoder, )?; decoder.start()?; Ok(decoder) } -// Creates a hardware (or assumed hardware) MediaCodec and then configure and start it. -fn decoder_try_hardware( - mime: &str, - format: &MediaFormat, - image_reader: &ImageReader, -) -> Result { - let tmp = MediaCodec::from_decoder_type(&mime) - .ok_or(anyhow!("unable to find decoder for mime type: {}", &mime))?; - decoder_configure_and_start(tmp, &format, &image_reader) -} - -// Creates a software MediaCodec and then configure and start it. -fn decoder_try_software( - codec_name: &str, - format: &MediaFormat, - image_reader: &ImageReader, -) -> Result { - let tmp = MediaCodec::from_codec_name(&codec_name) - .ok_or(anyhow!("no such codec: {}", &codec_name))?; - decoder_configure_and_start(tmp, &format, &image_reader) -} - // Create a sink/source pair pub fn video_decoder_split( config: DecoderInitConfig, @@ -259,15 +259,7 @@ pub fn video_decoder_split( .set_buffer_removed_listener(Box::new(|_, _| ())) .unwrap(); - let mime = match config.codec { - CodecType::H264 => "video/avc", - CodecType::Hevc => "video/hevc", - }; - - let sw_codec_name = match config.codec { - CodecType::H264 => "OMX.google.h264.decoder", - CodecType::Hevc => "OMX.google.hevc.decoder", - }; + let mime = mime_for_codec(config.codec); let format = MediaFormat::new(); format.set_str("mime", mime); @@ -287,23 +279,23 @@ pub fn video_decoder_split( info!("Using AMediaCoded format:{} ", format); let preparing_decoder = if config.force_software_decoder { - decoder_try_software(sw_codec_name, &format, &image_reader).unwrap() + 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_try_hardware(mime, &format, &image_reader) { - Ok(d) => d, + match decoder_attempt_setup(config.codec, false, &format, &image_reader) { + Ok(d) => Ok(d), Err(e) => { error!( "Attempting software fallback due to error in default decoder: {}", e ); - decoder_try_software(sw_codec_name, &format, &image_reader).unwrap() + decoder_attempt_setup(config.codec, true, &format, &image_reader) } } }; - let decoder = Arc::new(FakeThreadSafe(preparing_decoder)); + let decoder = Arc::new(FakeThreadSafe(preparing_decoder.unwrap())); { let mut decoder_lock = decoder_sink.lock(); From fc739bdd53fe21f378bfc0ef493b9a5b91057312 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Mon, 18 Dec 2023 20:48:23 +0000 Subject: [PATCH 4/9] Android: decoder.rs: cargo fmt --all --- alvr/client_core/src/platform/android/decoder.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index 392532954e..be100d85a2 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -144,9 +144,7 @@ impl Drop for VideoDecoderSource { } } -fn mime_for_codec( - codec: CodecType -) -> &'static str { +fn mime_for_codec(codec: CodecType) -> &'static str { match codec { CodecType::H264 => "video/avc", CodecType::Hevc => "video/hevc", From ebdc6adfaf8a5ce8f65110f094a31397ad3f74de Mon Sep 17 00:00:00 2001 From: 20kdc Date: Mon, 18 Dec 2023 21:54:07 +0000 Subject: [PATCH 5/9] Add option to force software decoding --- alvr/client_core/src/connection.rs | 2 +- alvr/client_core/src/decoder.rs | 3 ++- alvr/session/src/settings.rs | 6 ++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/alvr/client_core/src/connection.rs b/alvr/client_core/src/connection.rs index 4fae3ca12d..55af7356ce 100644 --- a/alvr/client_core/src/connection.rs +++ b/alvr/client_core/src/connection.rs @@ -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}"); diff --git a/alvr/client_core/src/decoder.rs b/alvr/client_core/src/decoder.rs index 2291fc2479..acceaa863a 100644 --- a/alvr/client_core/src/decoder.rs +++ b/alvr/client_core/src/decoder.rs @@ -38,9 +38,10 @@ pub static DECODER_SOURCE: alvr_common::OptLazy, + #[schema(strings( + help = "Attempts to use a software decoder on the device. Slow, but may work around broken codecs." + ))] + pub force_software_decoder: bool, + #[schema(flag = "steamvr-restart")] pub color_correction: Switch, } @@ -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 { From 97eff12fffb2a0909c6264ebc83546211e88dde8 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Wed, 20 Dec 2023 16:33:26 +0000 Subject: [PATCH 6/9] Android: Reduce use of unwrapping in decoder setup. Logging: errors should be more explicit --- alvr/client_core/src/decoder.rs | 7 -- .../src/platform/android/decoder.rs | 85 ++++++++++--------- alvr/common/src/logging.rs | 4 +- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/alvr/client_core/src/decoder.rs b/alvr/client_core/src/decoder.rs index acceaa863a..e1e5340155 100644 --- a/alvr/client_core/src/decoder.rs +++ b/alvr/client_core/src/decoder.rs @@ -7,8 +7,6 @@ use std::time::Duration; #[derive(Clone)] pub struct DecoderInitConfig { pub codec: CodecType, - pub width: i32, - pub height: i32, pub force_software_decoder: bool, pub max_buffering_frames: f32, pub buffering_history_weight: f32, @@ -18,11 +16,6 @@ pub struct DecoderInitConfig { pub static DECODER_INIT_CONFIG: Lazy> = Lazy::new(|| { Mutex::new(DecoderInitConfig { codec: CodecType::H264, - // TODO: Actually get the real values for these and update accordingly. - // These are inherited from the previous code to avoid breaking anything. - // Would also be nice if force_software_decoder was configurable. - width: 512, - height: 1024, force_software_decoder: false, max_buffering_frames: 1.0, buffering_history_weight: 0.9, diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index be100d85a2..463946e141 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -1,9 +1,9 @@ use crate::decoder::DecoderInitConfig; use alvr_common::{ - anyhow::{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::{ @@ -170,12 +170,14 @@ fn decoder_attempt_setup( 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, - )?; - decoder.start()?; + 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) } @@ -261,8 +263,10 @@ pub fn video_decoder_split( let format = MediaFormat::new(); format.set_str("mime", mime); - format.set_i32("width", config.width); - format.set_i32("height", config.height); + // 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 { @@ -284,8 +288,9 @@ pub fn video_decoder_split( 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: {}", + "Attempting software fallback due to error in default decoder: {:#}", e ); decoder_attempt_setup(config.codec, true, &format, &image_reader) @@ -293,43 +298,47 @@ pub fn video_decoder_split( } }; - let decoder = Arc::new(FakeThreadSafe(preparing_decoder.unwrap())); + if let Ok(prepared_decoder) = preparing_decoder { + let decoder = Arc::new(FakeThreadSafe(prepared_decoder)); - { - let mut decoder_lock = decoder_sink.lock(); + { + let mut decoder_lock = decoder_sink.lock(); - *decoder_lock = Some(Arc::clone(&decoder)); + *decoder_lock = Some(Arc::clone(&decoder)); - decoder_ready_notifier.notify_one(); - } + decoder_ready_notifier.notify_one(); + } - 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(); + 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}"); + 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); + } image_queue.lock().clear(); error!("FIXME: Leaking Imagereader!"); diff --git a/alvr/common/src/logging.rs b/alvr/common/src/logging.rs index a02f3cc877..bddd00d31a 100644 --- a/alvr/common/src/logging.rs +++ b/alvr/common/src/logging.rs @@ -128,11 +128,11 @@ pub fn show_e_blocking(e: E) { } pub fn show_err(res: Result) -> Option { - 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(res: Result) -> Option { - res.map_err(|e| show_e_block(e, true)).ok() + res.map_err(|e| show_e_block(format!("{:#}", e), true)).ok() } pub trait ToAny { From d9a8731457c6bf32e5c7dd6fd5490a68ae2146f2 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Wed, 20 Dec 2023 16:42:08 +0000 Subject: [PATCH 7/9] Settings: Move force_software_decoder above mediacodec_extra_options, Logging: cargo fmt --all --- alvr/common/src/logging.rs | 3 ++- alvr/session/src/settings.rs | 10 +++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/alvr/common/src/logging.rs b/alvr/common/src/logging.rs index bddd00d31a..d005c911b5 100644 --- a/alvr/common/src/logging.rs +++ b/alvr/common/src/logging.rs @@ -128,7 +128,8 @@ pub fn show_e_blocking(e: E) { } pub fn show_err(res: Result) -> Option { - res.map_err(|e| show_e_block(format!("{:#}", e), false)).ok() + res.map_err(|e| show_e_block(format!("{:#}", e), false)) + .ok() } pub fn show_err_blocking(res: Result) -> Option { diff --git a/alvr/session/src/settings.rs b/alvr/session/src/settings.rs index 92e4b2d43d..652ad0a91e 100644 --- a/alvr/session/src/settings.rs +++ b/alvr/session/src/settings.rs @@ -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")] @@ -479,11 +484,6 @@ pub struct VideoConfig { pub clientside_foveation: Switch, - #[schema(strings( - help = "Attempts to use a software decoder on the device. Slow, but may work around broken codecs." - ))] - pub force_software_decoder: bool, - #[schema(flag = "steamvr-restart")] pub color_correction: Switch, } From d3645feee654da71fa3ec9f7b49bb6e0c9e74845 Mon Sep 17 00:00:00 2001 From: 20kdc Date: Wed, 20 Dec 2023 21:50:50 +0000 Subject: [PATCH 8/9] Revert logging changes made for Android decoder error messages --- alvr/client_core/src/platform/android/decoder.rs | 16 ++++++++-------- alvr/common/src/logging.rs | 5 ++--- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index 463946e141..cd095300e5 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -170,14 +170,14 @@ fn decoder_attempt_setup( 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")?; + let decoder_configure_err = decoder.configure( + &format, + Some(&image_reader.window()?), + MediaCodecDirection::Decoder, + ); + decoder_configure_err.with_context(|| format!("failed to configure decoder"))?; + let decoder_start_err = decoder.start(); + decoder_start_err.with_context(|| format!("failed to start decoder"))?; Ok(decoder) } diff --git a/alvr/common/src/logging.rs b/alvr/common/src/logging.rs index d005c911b5..a02f3cc877 100644 --- a/alvr/common/src/logging.rs +++ b/alvr/common/src/logging.rs @@ -128,12 +128,11 @@ pub fn show_e_blocking(e: E) { } pub fn show_err(res: Result) -> Option { - res.map_err(|e| show_e_block(format!("{:#}", e), false)) - .ok() + res.map_err(|e| show_e_block(e, false)).ok() } pub fn show_err_blocking(res: Result) -> Option { - res.map_err(|e| show_e_block(format!("{:#}", e), true)).ok() + res.map_err(|e| show_e_block(e, true)).ok() } pub trait ToAny { From 41bb3a79b3ef442b25b988c262984f65fbfe181d Mon Sep 17 00:00:00 2001 From: 20kdc Date: Wed, 20 Dec 2023 22:12:27 +0000 Subject: [PATCH 9/9] Android: Use match instead of if-let for decoder error handling --- .../src/platform/android/decoder.rs | 69 ++++++++++--------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/alvr/client_core/src/platform/android/decoder.rs b/alvr/client_core/src/platform/android/decoder.rs index cd095300e5..fe8318f1fd 100644 --- a/alvr/client_core/src/platform/android/decoder.rs +++ b/alvr/client_core/src/platform/android/decoder.rs @@ -3,7 +3,7 @@ use alvr_common::{ anyhow::{anyhow, bail, Context, Result}, error, info, parking_lot::{Condvar, Mutex}, - show_err, warn, RelaxedAtomic, + show_e, warn, RelaxedAtomic, }; use alvr_session::{CodecType, MediacodecDataType}; use ndk::{ @@ -298,46 +298,51 @@ pub fn video_decoder_split( } }; - if let Ok(prepared_decoder) = preparing_decoder { - let decoder = Arc::new(FakeThreadSafe(prepared_decoder)); + match preparing_decoder { + Ok(prepared_decoder) => { + let decoder = Arc::new(FakeThreadSafe(prepared_decoder)); - { - let mut decoder_lock = decoder_sink.lock(); + { + let mut decoder_lock = decoder_sink.lock(); - *decoder_lock = Some(Arc::clone(&decoder)); + *decoder_lock = Some(Arc::clone(&decoder)); - decoder_ready_notifier.notify_one(); - } - - 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); - } else { - show_err(preparing_decoder); + // Destroy all resources + decoder_sink.lock().take(); // Make sure the shared ref is deleted first + decoder.stop().unwrap(); + drop(decoder); + } + Err(e) => { + show_e(e); + } } image_queue.lock().clear();