From 8b5d3bc0fd6b34ee5f817a62d4ebc30843a43987 Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 20:34:11 +0200 Subject: [PATCH 1/7] clean up listen() --- src/ipc.rs | 172 +++++++++++++++++++---------------------------------- 1 file changed, 61 insertions(+), 111 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index 5f78333..8558df3 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -338,125 +338,75 @@ pub fn listen(instance: &mut Mpv) -> Result { instance.reader.read_line(&mut response).unwrap(); response = response.trim_end().to_string(); debug!("Event: {}", response); - match serde_json::from_str::(&response) { - Ok(e) => { - if let Value::String(ref name) = e["event"] { - let event: Event; - match name.as_str() { - "shutdown" => { - event = Event::Shutdown; - } - "start-file" => { - event = Event::StartFile; - } - "file-loaded" => { - event = Event::FileLoaded; - } - "seek" => { - event = Event::Seek; - } - "playback-restart" => { - event = Event::PlaybackRestart; - } - "idle" => { - event = Event::Idle; - } - "tick" => { - event = Event::Tick; - } - "video-reconfig" => { - event = Event::VideoReconfig; - } - "audio-reconfig" => { - event = Event::AudioReconfig; - } - "tracks-changed" => { - event = Event::TracksChanged; - } - "track-switched" => { - event = Event::TrackSwitched; - } - "pause" => { - event = Event::Pause; - } - "unpause" => { - event = Event::Unpause; - } - "metadata-update" => { - event = Event::MetadataUpdate; - } - "chapter-change" => { - event = Event::ChapterChange; - } - "end-file" => { - event = Event::EndFile; - } - "property-change" => { - let name: String; - let id: usize; - let data: MpvDataType; - if let Value::String(ref n) = e["name"] { - name = n.to_string(); + let e = serde_json::from_str::(&response) + .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; + + if let Value::String(ref name) = e["event"] { + let event = match name.as_str() { + "shutdown" => Event::Shutdown, + "start-file" => Event::StartFile, + "file-loaded" => Event::FileLoaded, + "seek" => Event::Seek, + "playback-restart" => Event::PlaybackRestart, + "idle" => Event::Idle, + "tick" => Event::Tick, + "video-reconfig" => Event::VideoReconfig, + "audio-reconfig" => Event::AudioReconfig, + "tracks-changed" => Event::TracksChanged, + "track-switched" => Event::TrackSwitched, + "pause" => Event::Pause, + "unpause" => Event::Unpause, + "metadata-update" => Event::MetadataUpdate, + "chapter-change" => Event::ChapterChange, + "end-file" => Event::EndFile, + "property-change" => { + let name = match e["name"] { + Value::String(ref n) => Ok(n.to_string()), + _ => Err(Error(ErrorCode::JsonContainsUnexptectedType)), + }?; + + let id: usize = match e["id"] { + Value::Number(ref n) => n.as_u64().unwrap() as usize, + _ => 0, + }; + + let data: MpvDataType = match e["data"] { + Value::String(ref n) => MpvDataType::String(n.to_string()), + + Value::Array(ref a) => { + if name == "playlist".to_string() { + MpvDataType::Playlist(Playlist(json_array_to_playlist(a))) + } else { + MpvDataType::Array(json_array_to_vec(a)) + } + } + + Value::Bool(b) => MpvDataType::Bool(b), + + Value::Number(ref n) => { + if n.is_u64() { + MpvDataType::Usize(n.as_u64().unwrap() as usize) + } else if n.is_f64() { + MpvDataType::Double(n.as_f64().unwrap()) } else { return Err(Error(ErrorCode::JsonContainsUnexptectedType)); } - - if let Value::Number(ref n) = e["id"] { - id = n.as_i64().unwrap() as usize; - } else { - id = 0; - } - - match e["data"] { - Value::String(ref n) => { - data = MpvDataType::String(n.to_string()); - } - - Value::Array(ref a) => { - if name == "playlist".to_string() { - data = - MpvDataType::Playlist(Playlist(json_array_to_playlist(a))); - } else { - data = MpvDataType::Array(json_array_to_vec(a)); - } - } - - Value::Bool(ref b) => { - data = MpvDataType::Bool(*b); - } - - Value::Number(ref n) => { - if n.is_u64() { - data = MpvDataType::Usize(n.as_u64().unwrap() as usize); - } else if n.is_f64() { - data = MpvDataType::Double(n.as_f64().unwrap()); - } else { - return Err(Error(ErrorCode::JsonContainsUnexptectedType)); - } - } - - Value::Object(ref m) => { - data = MpvDataType::HashMap(json_map_to_hashmap(m)); - } - - Value::Null => { - data = MpvDataType::Null; - } - } - - event = try_convert_property(name.as_ref(), id, data); - } - _ => { - event = Event::Unimplemented; } + + Value::Object(ref m) => MpvDataType::HashMap(json_map_to_hashmap(m)), + + Value::Null => MpvDataType::Null, }; - return Ok(event); + + try_convert_property(name.as_ref(), id, data) } - } - Err(why) => return Err(Error(ErrorCode::JsonParseError(why.to_string()))), + _ => Event::Unimplemented, + }; + Ok(event) + } else { + unreachable!(); } - unreachable!(); } pub fn listen_raw(instance: &mut Mpv) -> String { From fde2bce07dd2170b76aa433859591408154390eb Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 20:54:44 +0200 Subject: [PATCH 2/7] clean up get_mpv_property_string() --- src/ipc.rs | 51 ++++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index 8558df3..50bfb5f 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -189,30 +189,35 @@ pub fn get_mpv_property(instance: &Mpv, property: &str) -> Resul pub fn get_mpv_property_string(instance: &Mpv, property: &str) -> Result { let ipc_string = format!("{{ \"command\": [\"get_property\",\"{}\"] }}\n", property); - match serde_json::from_str::(&send_command_sync(instance, &ipc_string)) { - Ok(val) => { - if let Value::Object(map) = val { - if let Value::String(ref error) = map["error"] { - if error == "success" && map.contains_key("data") { - match map["data"] { - Value::Bool(b) => Ok(b.to_string()), - Value::Number(ref n) => Ok(n.to_string()), - Value::String(ref s) => Ok(s.to_string()), - Value::Array(ref array) => Ok(format!("{:?}", array)), - Value::Object(ref map) => Ok(format!("{:?}", map)), - _ => Err(Error(ErrorCode::UnsupportedType)), - } - } else { - Err(Error(ErrorCode::MpvError(error.to_string()))) - } - } else { - Err(Error(ErrorCode::UnexpectedValue)) - } - } else { - Err(Error(ErrorCode::UnexpectedValue)) - } + let val = serde_json::from_str::(&send_command_sync(instance, &ipc_string)) + .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; + + let map = if let Value::Object(map) = val { + Ok(map) + } else { + Err(Error(ErrorCode::UnexpectedValue)) + }?; + + let error = if let Value::String(ref error) = map["error"] { + Ok(error) + } else { + Err(Error(ErrorCode::UnexpectedValue)) + }?; + + if error == "success" && map.contains_key("data") { + match map["data"] { + Value::Bool(b) => Ok(b.to_string()), + Value::Number(ref n) => Ok(n.to_string()), + Value::String(ref s) => Ok(s.to_string()), + Value::Array(ref array) => Ok(format!("{:?}", array)), + Value::Object(ref map) => Ok(format!("{:?}", map)), + _ => Err(Error(ErrorCode::UnsupportedType)), } - Err(why) => Err(Error(ErrorCode::JsonParseError(why.to_string()))), + } else { + // TODO: there is a bug here + // this could return MpvError("success") if error == "success" but the map doesn't contain + // data + Err(Error(ErrorCode::MpvError(error.to_string()))) } } From cc9f2cae53f22ab99bba9fca5420fad2a729f854 Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 20:59:33 +0200 Subject: [PATCH 3/7] fix potential bug could potentially return MpvError("success") --- src/ipc.rs | 23 +++++++++++------------ src/lib.rs | 2 ++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index 50bfb5f..055fa47 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -204,20 +204,19 @@ pub fn get_mpv_property_string(instance: &Mpv, property: &str) -> Result Ok(b.to_string()), - Value::Number(ref n) => Ok(n.to_string()), - Value::String(ref s) => Ok(s.to_string()), - Value::Array(ref array) => Ok(format!("{:?}", array)), - Value::Object(ref map) => Ok(format!("{:?}", map)), - _ => Err(Error(ErrorCode::UnsupportedType)), - } + let data = if error == "success" { + Ok(&map["data"]) } else { - // TODO: there is a bug here - // this could return MpvError("success") if error == "success" but the map doesn't contain - // data Err(Error(ErrorCode::MpvError(error.to_string()))) + }?; + + match data { + Value::Bool(b) => Ok(b.to_string()), + Value::Number(ref n) => Ok(n.to_string()), + Value::String(ref s) => Ok(s.to_string()), + Value::Array(ref array) => Ok(format!("{:?}", array)), + Value::Object(ref map) => Ok(format!("{:?}", map)), + Value::Null => Err(Error(ErrorCode::MissingValue)), } } diff --git a/src/lib.rs b/src/lib.rs index 5bb122e..d38ef29 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -118,6 +118,7 @@ pub enum ErrorCode { JsonContainsUnexptectedType, UnexpectedResult, UnexpectedValue, + MissingValue, UnsupportedType, ValueDoesNotContainBool, ValueDoesNotContainF64, @@ -190,6 +191,7 @@ impl Display for ErrorCode { } ErrorCode::UnexpectedResult => f.write_str("Unexpected result received"), ErrorCode::UnexpectedValue => f.write_str("Unexpected value received"), + ErrorCode::MissingValue => f.write_str("Missing value"), ErrorCode::UnsupportedType => f.write_str("Unsupported type received"), ErrorCode::ValueDoesNotContainBool => { f.write_str("The received value is not of type \'std::bool\'") From be2132735b381c9b0709734737104683d084e351 Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 21:06:34 +0200 Subject: [PATCH 4/7] remove comments --- src/ipc.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index 055fa47..e2b3488 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -417,10 +417,6 @@ pub fn listen_raw(instance: &mut Mpv) -> String { let mut response = String::new(); instance.reader.read_line(&mut response).unwrap(); response.trim_end().to_string() - // let mut stream = &instance.0; - // let mut buffer = [0; 32]; - // stream.read(&mut buffer[..]).unwrap(); - // String::from_utf8_lossy(&buffer).into_owned() } fn send_command_sync(instance: &Mpv, command: &str) -> String { From 71b148cce4f28aee3100e1fb3fc6e7976fb58d42 Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 21:06:47 +0200 Subject: [PATCH 5/7] auto formatting --- src/lib.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d38ef29..31e5017 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -54,7 +54,7 @@ pub enum MpvCommand { }, Observe { id: isize, - property: String + property: String, }, PlaylistNext, PlaylistPrev, @@ -488,9 +488,7 @@ impl Mpv { }, ], ), - MpvCommand::Observe { id, property } => { - observe_mpv_property(self, &id, &property) - } + MpvCommand::Observe { id, property } => observe_mpv_property(self, &id, &property), MpvCommand::PlaylistClear => run_mpv_command(self, "playlist-clear", &[]), MpvCommand::PlaylistMove { from, to } => { run_mpv_command(self, "playlist-move", &[&from.to_string(), &to.to_string()]) @@ -516,9 +514,7 @@ impl Mpv { ], ), MpvCommand::Stop => run_mpv_command(self, "stop", &[]), - MpvCommand::Unobserve(id) => { - unobserve_mpv_property(self, &id) - } + MpvCommand::Unobserve(id) => unobserve_mpv_property(self, &id), } } From fded248b7e7a048afec67c393c0ec7d2d623adbe Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 21:07:20 +0200 Subject: [PATCH 6/7] minor cleanup: run_mpv_command() --- src/ipc.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index e2b3488..5fa171a 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -237,14 +237,11 @@ pub fn set_mpv_property( } pub fn run_mpv_command(instance: &Mpv, command: &str, args: &[&str]) -> Result<(), Error> { - let mut ipc_string = format!("{{ \"command\": [\"{}\"", command); - if args.len() > 0 { - for arg in args { - ipc_string.push_str(&format!(", \"{}\"", arg)); - } + let mut ipc_string = format!(r#"{{ "command": ["{}""#, command); + for arg in args { + ipc_string.push_str(&format!(r#", "{}""#, arg)); } ipc_string.push_str("] }\n"); - ipc_string = ipc_string; match serde_json::from_str::(&send_command_sync(instance, &ipc_string)) { Ok(feedback) => { if let Value::String(ref error) = feedback["error"] { From 9fde540089a0778686ab08dd90df1f8ead89c867 Mon Sep 17 00:00:00 2001 From: jole Date: Tue, 19 Jul 2022 21:27:02 +0200 Subject: [PATCH 7/7] do not panic on message without event field intead discard it and try again --- src/ipc.rs | 149 ++++++++++++++++++++++++++++------------------------- 1 file changed, 79 insertions(+), 70 deletions(-) diff --git a/src/ipc.rs b/src/ipc.rs index 5fa171a..670dc53 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -335,79 +335,88 @@ fn try_convert_property(name: &str, id: usize, data: MpvDataType) -> Event { } pub fn listen(instance: &mut Mpv) -> Result { - let mut response = String::new(); - instance.reader.read_line(&mut response).unwrap(); - response = response.trim_end().to_string(); - debug!("Event: {}", response); + let mut e; + // sometimes we get responses unrelated to events, so we read a new line until we receive one + // with an event field + let name = loop { + let mut response = String::new(); + instance.reader.read_line(&mut response).unwrap(); + response = response.trim_end().to_string(); + debug!("Event: {}", response); - let e = serde_json::from_str::(&response) - .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; + e = serde_json::from_str::(&response) + .map_err(|why| Error(ErrorCode::JsonParseError(why.to_string())))?; - if let Value::String(ref name) = e["event"] { - let event = match name.as_str() { - "shutdown" => Event::Shutdown, - "start-file" => Event::StartFile, - "file-loaded" => Event::FileLoaded, - "seek" => Event::Seek, - "playback-restart" => Event::PlaybackRestart, - "idle" => Event::Idle, - "tick" => Event::Tick, - "video-reconfig" => Event::VideoReconfig, - "audio-reconfig" => Event::AudioReconfig, - "tracks-changed" => Event::TracksChanged, - "track-switched" => Event::TrackSwitched, - "pause" => Event::Pause, - "unpause" => Event::Unpause, - "metadata-update" => Event::MetadataUpdate, - "chapter-change" => Event::ChapterChange, - "end-file" => Event::EndFile, - "property-change" => { - let name = match e["name"] { - Value::String(ref n) => Ok(n.to_string()), - _ => Err(Error(ErrorCode::JsonContainsUnexptectedType)), - }?; - - let id: usize = match e["id"] { - Value::Number(ref n) => n.as_u64().unwrap() as usize, - _ => 0, - }; - - let data: MpvDataType = match e["data"] { - Value::String(ref n) => MpvDataType::String(n.to_string()), - - Value::Array(ref a) => { - if name == "playlist".to_string() { - MpvDataType::Playlist(Playlist(json_array_to_playlist(a))) - } else { - MpvDataType::Array(json_array_to_vec(a)) - } - } - - Value::Bool(b) => MpvDataType::Bool(b), - - Value::Number(ref n) => { - if n.is_u64() { - MpvDataType::Usize(n.as_u64().unwrap() as usize) - } else if n.is_f64() { - MpvDataType::Double(n.as_f64().unwrap()) - } else { - return Err(Error(ErrorCode::JsonContainsUnexptectedType)); - } - } - - Value::Object(ref m) => MpvDataType::HashMap(json_map_to_hashmap(m)), - - Value::Null => MpvDataType::Null, - }; - - try_convert_property(name.as_ref(), id, data) + match e["event"] { + Value::String(ref name) => break name, + _ => { + // It was not an event - try again + debug!("Bad response: {:?}", response) } - _ => Event::Unimplemented, - }; - Ok(event) - } else { - unreachable!(); - } + } + }; + + let event = match name.as_str() { + "shutdown" => Event::Shutdown, + "start-file" => Event::StartFile, + "file-loaded" => Event::FileLoaded, + "seek" => Event::Seek, + "playback-restart" => Event::PlaybackRestart, + "idle" => Event::Idle, + "tick" => Event::Tick, + "video-reconfig" => Event::VideoReconfig, + "audio-reconfig" => Event::AudioReconfig, + "tracks-changed" => Event::TracksChanged, + "track-switched" => Event::TrackSwitched, + "pause" => Event::Pause, + "unpause" => Event::Unpause, + "metadata-update" => Event::MetadataUpdate, + "chapter-change" => Event::ChapterChange, + "end-file" => Event::EndFile, + "property-change" => { + let name = match e["name"] { + Value::String(ref n) => Ok(n.to_string()), + _ => Err(Error(ErrorCode::JsonContainsUnexptectedType)), + }?; + + let id: usize = match e["id"] { + Value::Number(ref n) => n.as_u64().unwrap() as usize, + _ => 0, + }; + + let data: MpvDataType = match e["data"] { + Value::String(ref n) => MpvDataType::String(n.to_string()), + + Value::Array(ref a) => { + if name == "playlist".to_string() { + MpvDataType::Playlist(Playlist(json_array_to_playlist(a))) + } else { + MpvDataType::Array(json_array_to_vec(a)) + } + } + + Value::Bool(b) => MpvDataType::Bool(b), + + Value::Number(ref n) => { + if n.is_u64() { + MpvDataType::Usize(n.as_u64().unwrap() as usize) + } else if n.is_f64() { + MpvDataType::Double(n.as_f64().unwrap()) + } else { + return Err(Error(ErrorCode::JsonContainsUnexptectedType)); + } + } + + Value::Object(ref m) => MpvDataType::HashMap(json_map_to_hashmap(m)), + + Value::Null => MpvDataType::Null, + }; + + try_convert_property(name.as_ref(), id, data) + } + _ => Event::Unimplemented, + }; + Ok(event) } pub fn listen_raw(instance: &mut Mpv) -> String {