Skip to content

Commit

Permalink
fix: content loading related crashes (android) (#168)
Browse files Browse the repository at this point in the history
* fix: re-render ui layout when input/dropdown changes

* add content notificator

* skip dependencies when it fails

* test with tokio

* fix and enable android build

* fix tooltip when IA_ANY is specified
move memcpy to packed array to helper functions (fast_create_packed_byte_array_from_*)
remove nop()
mock ~system/CommunicationsController::sendBinary

* workaround to resolve promises, fix blocking when scene is loaded

* rewrite: prettier content-provider code removing several if Err(_)

* restore ci
  • Loading branch information
leanmendoza authored Jan 16, 2024
1 parent 8700db5 commit fb785e6
Show file tree
Hide file tree
Showing 22 changed files with 423 additions and 480 deletions.
4 changes: 4 additions & 0 deletions godot/src/test/testing_api.gd
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ func async_take_and_compare_snapshot(
dcl_rpc_sender
)

var pending_promises := Global.content_provider.get_pending_promises()
if not pending_promises.is_empty():
await PromiseUtils.async_all(Global.content_provider.get_pending_promises())

# TODO: make this configurable
var hide_player := true

Expand Down
18 changes: 13 additions & 5 deletions godot/src/ui/components/pointer_tooltip/tooltip_label.gd
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ func _ready():

func set_tooltip_data(text: String, action: String):
var key: String
var index: int = InputMap.get_actions().find(action.to_lower(), 0)
var action_lower: String = action.to_lower()
var index: int = InputMap.get_actions().find(action_lower, 0)
if label_text:
if index != -1:
action_to_trigger = action.to_lower()
show()
if index == -1 and action_lower == "ia_any":
key = "Any"
elif index != -1:
var event = InputMap.action_get_events(InputMap.get_actions()[index])[0]
if event is InputEventKey:
key = char(event.unicode).to_upper()
Expand All @@ -29,15 +30,22 @@ func set_tooltip_data(text: String, action: String):
key = "Mouse Right Button"
if event.button_index == 0:
key = "Mouse Wheel Button"

if not key.is_empty():
show()
action_to_trigger = action_lower
label_action.text = key
label_text.text = text
else:
action_to_trigger = ""
hide()
action_to_trigger = ""
printerr("Action doesn't exist ", action)


func mobile_on_panel_container_gui_input(event):
if action_to_trigger.is_empty():
return

if event is InputEventMouseButton:
if event.pressed:
Input.action_press(action_to_trigger)
Expand Down
17 changes: 2 additions & 15 deletions rust/decentraland-godot-lib/src/auth/dcl_player_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rand::thread_rng;
use tokio::task::JoinHandle;

use crate::comms::profile::{LambdaProfiles, UserProfile};
use crate::content::bytes::fast_create_packed_byte_array_from_vec;
use crate::dcl::scene_apis::RpcResultSender;
use crate::godot_classes::promise::Promise;
use crate::http_request::request_response::{RequestResponse, ResponseEnum};
Expand Down Expand Up @@ -388,21 +389,7 @@ impl DclPlayerIdentity {
return;
};

// TODO: gdext should implement a packedByteArray constructor from &[u8] and not iteration
let body_payload = {
let byte_length = body_payload.len();
let mut param = PackedByteArray::new();
param.resize(byte_length);
let data_arr_ptr = param.as_mut_slice();

unsafe {
let dst_ptr = &mut data_arr_ptr[0] as *mut u8;
let src_ptr = &body_payload[0] as *const u8;
std::ptr::copy_nonoverlapping(src_ptr, dst_ptr, byte_length);
}
param
};

let body_payload = fast_create_packed_byte_array_from_vec(&body_payload);
let mut dict = Dictionary::default();
dict.set("content_type", content_type.to_variant());
dict.set("body_payload", body_payload.to_variant());
Expand Down
16 changes: 4 additions & 12 deletions rust/decentraland-godot-lib/src/av/video_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@ use ffmpeg_next::software::scaling::{context::Context, flag::Flags};
use ffmpeg_next::{decoder, format::context::Input, media::Type, util::frame, Packet};
use godot::engine::image::Format;
use godot::engine::{Image, ImageTexture};
use godot::prelude::{Gd, PackedByteArray, Vector2};
use godot::prelude::{Gd, Vector2};
use thiserror::Error;
use tracing::debug;

use crate::content::bytes::fast_create_packed_byte_array_from_slice;

use super::stream_processor::FfmpegContext;

pub struct VideoInfo {
Expand Down Expand Up @@ -150,17 +152,7 @@ impl FfmpegContext for VideoContext {
// let data_arr = PackedByteArray::from(current_frame.data(0));

let raw_data = current_frame.data(0);
let byte_length = raw_data.len();
let mut data_arr = PackedByteArray::new();
data_arr.resize(raw_data.len());

let data_arr_ptr = data_arr.as_mut_slice();

unsafe {
let dst_ptr = &mut data_arr_ptr[0] as *mut u8;
let src_ptr = &raw_data[0] as *const u8;
std::ptr::copy_nonoverlapping(src_ptr, dst_ptr, byte_length);
}
let data_arr = fast_create_packed_byte_array_from_slice(raw_data);

let diff = self.last_frame_time.elapsed().as_secs_f32();
debug!(
Expand Down
101 changes: 30 additions & 71 deletions rust/decentraland-godot-lib/src/content/audio.rs
Original file line number Diff line number Diff line change
@@ -1,91 +1,57 @@
use godot::{
builtin::{meta::ToGodot, GString},
engine::{file_access::ModeFlags, AudioStream, AudioStreamMp3, AudioStreamWav, FileAccess},
builtin::{meta::ToGodot, Variant},
engine::{AudioStream, AudioStreamMp3, AudioStreamOggVorbis, AudioStreamWav},
obj::Gd,
};

use crate::{
godot_classes::promise::Promise,
http_request::request_response::{RequestOption, ResponseType},
};
use tokio::io::AsyncReadExt;

use super::{
content_mapping::ContentMappingAndUrlRef,
content_provider::ContentProviderContext,
file_string::get_extension,
thread_safety::{reject_promise, resolve_promise},
bytes::fast_create_packed_byte_array_from_vec, content_mapping::ContentMappingAndUrlRef,
content_provider::ContentProviderContext, download::fetch_resource_or_wait,
file_string::get_extension, thread_safety::GodotSingleThreadSafety,
};

pub async fn load_audio(
file_path: String,
content_mapping: ContentMappingAndUrlRef,
get_promise: impl Fn() -> Option<Gd<Promise>>,
ctx: ContentProviderContext,
) {
) -> Result<Option<Variant>, anyhow::Error> {
let extension = get_extension(&file_path);
if ["wav", "ogg", "mp3"].contains(&extension.as_str()) {
reject_promise(
get_promise,
format!("Audio {} unrecognized format", file_path),
);
return;
return Err(anyhow::Error::msg(format!(
"Audio {} unrecognized format",
file_path
)));
}

let Some(file_hash) = content_mapping.content.get(&file_path) else {
reject_promise(
get_promise,
"File not found in the content mappings".to_string(),
);
return;
};
let file_hash = content_mapping
.content
.get(&file_path)
.ok_or(anyhow::Error::msg("File not found in the content mappings"))?;

let url = format!("{}{}", content_mapping.base_url, file_hash);
let absolute_file_path = format!("{}{}", ctx.content_folder, file_hash);
if !FileAccess::file_exists(GString::from(&absolute_file_path)) {
let request = RequestOption::new(
0,
format!("{}{}", content_mapping.base_url, file_hash),
http::Method::GET,
ResponseType::ToFile(absolute_file_path.clone()),
None,
None,
None,
);

match ctx.http_queue_requester.request(request, 0).await {
Ok(_response) => {}
Err(err) => {
reject_promise(
get_promise,
format!(
"Error downloading audio {file_hash} ({file_path}): {:?}",
err
),
);
return;
}
}
}
fetch_resource_or_wait(&url, file_hash, &absolute_file_path, ctx.clone())
.await
.map_err(anyhow::Error::msg)?;

let Some(file) = FileAccess::open(GString::from(&absolute_file_path), ModeFlags::READ) else {
reject_promise(
get_promise,
format!("Error opening audio file {}", absolute_file_path),
);
return;
};
let mut file = tokio::fs::File::open(&absolute_file_path).await?;
let mut bytes_vec = Vec::new();
file.read_to_end(&mut bytes_vec).await?;

let _thread_safe_check = GodotSingleThreadSafety::acquire_owned(&ctx)
.await
.ok_or(anyhow::Error::msg("Failed while trying to "))?;

let bytes = file.get_buffer(file.get_length() as i64);
let bytes = fast_create_packed_byte_array_from_vec(&bytes_vec);
let audio_stream: Option<Gd<AudioStream>> = match extension.as_str() {
".wav" => {
let mut audio_stream = AudioStreamWav::new();
audio_stream.set_data(bytes);
Some(audio_stream.upcast())
}
// ".ogg" => {
// let audio_stream = AudioStreamOggVorbis::new();
// // audio_stream.set_(bytes);
// audio_stream.upcast()
// }
".ogg" => AudioStreamOggVorbis::load_from_buffer(bytes).map(|value| value.upcast()),
".mp3" => {
let mut audio_stream = AudioStreamMp3::new();
audio_stream.set_data(bytes);
Expand All @@ -94,13 +60,6 @@ pub async fn load_audio(
_ => None,
};

let Some(audio_stream) = audio_stream else {
reject_promise(
get_promise,
format!("Error creating audio stream for {}", absolute_file_path),
);
return;
};

resolve_promise(get_promise, Some(audio_stream.to_variant()));
let audio_stream = audio_stream.ok_or(anyhow::Error::msg("Error creating audio stream"))?;
Ok(Some(audio_stream.to_variant()))
}
21 changes: 21 additions & 0 deletions rust/decentraland-godot-lib/src/content/bytes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
use godot::builtin::PackedByteArray;

// TODO: gdext should implement a packedByteArray constructor from &[u8] and not iteration
pub fn fast_create_packed_byte_array_from_slice(bytes_slice: &[u8]) -> PackedByteArray {
let byte_length = bytes_slice.len();
let mut bytes = PackedByteArray::new();
bytes.resize(byte_length);

let data_arr_ptr = bytes.as_mut_slice();
unsafe {
let dst_ptr = &mut data_arr_ptr[0] as *mut u8;
let src_ptr = &bytes_slice[0] as *const u8;
std::ptr::copy_nonoverlapping(src_ptr, dst_ptr, byte_length);
}

bytes
}

pub fn fast_create_packed_byte_array_from_vec(bytes_vec: &Vec<u8>) -> PackedByteArray {
fast_create_packed_byte_array_from_slice(bytes_vec.as_slice())
}
36 changes: 25 additions & 11 deletions rust/decentraland-godot-lib/src/content/content_notificator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@ use std::{collections::HashMap, sync::Arc};

use tokio::sync::{Notify, RwLock};

#[derive(Clone, Debug)]
pub enum ContentState {
RequestOwner,
Busy(Arc<Notify>),
Released(Result<(), String>),
}

pub struct ContentNotificator {
files: RwLock<HashMap<String, Arc<Notify>>>,
files: RwLock<HashMap<String, ContentState>>,
}

impl Default for ContentNotificator {
Expand All @@ -19,24 +26,31 @@ impl ContentNotificator {
}
}

pub async fn get_or_create_notify(&self, key: String) -> (bool, Arc<Notify>) {
pub async fn get(&self, key: &String) -> Option<ContentState> {
let files = self.files.read().await;
files.get(key).cloned()
}

pub async fn get_or_create_notify(&self, key: &String) -> ContentState {
{
let files = self.files.read().await;
if let Some(notify) = files.get(&key) {
return (false, notify.clone());
if let Some(content_state) = files.get(key) {
return content_state.clone();
}
}

let mut files = self.files.write().await;
let notify = Arc::new(Notify::new());
files.insert(key, notify.clone());
drop(files);

(true, notify)
let content_state = ContentState::Busy(Arc::new(Notify::new()));
files.insert(key.clone(), content_state.clone());
ContentState::RequestOwner
}

pub async fn remove_notify(&mut self, key: String) {
pub async fn resolve(&self, key: &str, result: Result<(), String>) {
let mut files = self.files.write().await;
files.remove(&key);
if let Some(ContentState::Busy(notify)) =
files.insert(key.to_owned(), ContentState::Released(result))
{
notify.notify_waiters();
}
}
}
Loading

0 comments on commit fb785e6

Please sign in to comment.