diff --git a/src/media_player.rs b/src/media_player.rs index ee38b4c..2156060 100644 --- a/src/media_player.rs +++ b/src/media_player.rs @@ -9,10 +9,12 @@ use crate::EventManager; use libc::{c_void, c_uint}; use crate::enums::{State, Position}; use std::mem::transmute; +use std::sync::Mutex; /// A LibVLC media player plays one media (usually in a custom drawable). pub struct MediaPlayer { pub(crate) ptr: *mut sys::libvlc_media_player_t, + audio_callbacks: Mutex>>, } unsafe impl Send for MediaPlayer {} @@ -26,7 +28,10 @@ impl MediaPlayer { if p.is_null() { return None; } - Some(MediaPlayer{ptr: p}) + Some(MediaPlayer{ + ptr: p, + audio_callbacks: Mutex::new(AudioCallbackSlot::new()), + }) } } @@ -101,13 +106,17 @@ impl MediaPlayer { let flag_flush = flush.is_some(); let flag_drain = drain.is_some(); - let data = AudioCallbacksData { + let data = Box::new(AudioCallbacksData { play: Box::new(play), pause: pause, resume: resume, flush: flush, drain: drain, - }; - let data = Box::into_raw(Box::new(data)); + }); + let mut callbacks = self.audio_callbacks.lock().unwrap(); unsafe{ + // libVLC must stop using the previous opaque data before it is dropped. + callbacks.replace_after_unregistration(|| unset_audio_callbacks(self.ptr), data); + + let data = callbacks.callback().unwrap().opaque(); sys::libvlc_audio_set_callbacks( self.ptr, Some(audio_cb_play), @@ -115,7 +124,7 @@ impl MediaPlayer { if flag_resume {Some(audio_cb_resume)} else {None}, if flag_flush {Some(audio_cb_flush)} else {None}, if flag_drain {Some(audio_cb_drain)} else {None}, - data as *mut c_void); + data); } } @@ -322,10 +331,48 @@ impl MediaPlayer { impl Drop for MediaPlayer { fn drop(&mut self) { + self.audio_callbacks.get_mut().unwrap().clear_after_unregistration(|| { + // The callback data is owned by `audio_callbacks`, so unregister it first. + unset_audio_callbacks(self.ptr); + }); unsafe{ sys::libvlc_media_player_release(self.ptr) }; } } +fn unset_audio_callbacks(player: *mut sys::libvlc_media_player_t) { + unsafe{ + sys::libvlc_audio_set_callbacks(player, None, None, None, None, None, ::std::ptr::null_mut()); + } +} + +struct AudioCallbackSlot { + callback: Option, +} + +impl AudioCallbackSlot { + fn new() -> AudioCallbackSlot { + AudioCallbackSlot { callback: None } + } + + fn replace_after_unregistration(&mut self, unregister: F, callback: T) + where F: FnOnce(), + { + unregister(); + drop(self.callback.replace(callback)); + } + + fn clear_after_unregistration(&mut self, unregister: F) + where F: FnOnce(), + { + unregister(); + drop(self.callback.take()); + } + + fn callback(&self) -> Option<&T> { + self.callback.as_ref() + } +} + // For audio_set_callbacks struct AudioCallbacksData { play: Box, @@ -335,6 +382,12 @@ struct AudioCallbacksData { drain: Option>, } +impl AudioCallbacksData { + fn opaque(&self) -> *mut c_void { + self as *const AudioCallbacksData as *mut c_void + } +} + unsafe extern "C" fn audio_cb_play( data: *mut c_void, samples: *const c_void, count: c_uint, pts: i64) { let data: &AudioCallbacksData = transmute(data as *mut AudioCallbacksData); @@ -362,9 +415,48 @@ unsafe extern "C" fn audio_cb_drain(data: *mut c_void) { (data.drain.as_ref().unwrap())(); } +#[cfg(test)] +mod tests { + use super::AudioCallbackSlot; + use std::sync::Arc; + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + + struct DropCounter { + unregistered: Arc, + drops: Arc, + } + + impl Drop for DropCounter { + fn drop(&mut self) { + assert!(self.unregistered.load(Ordering::SeqCst)); + self.drops.fetch_add(1, Ordering::SeqCst); + } + } + + #[test] + fn audio_callback_slot_unregisters_before_replacing_and_clearing_callbacks() { + let unregistered = Arc::new(AtomicBool::new(false)); + let drops = Arc::new(AtomicUsize::new(0)); + let mut callbacks = AudioCallbackSlot::new(); + + callbacks.replace_after_unregistration( + || unregistered.store(true, Ordering::SeqCst), + DropCounter { unregistered: Arc::clone(&unregistered), drops: Arc::clone(&drops) }); + unregistered.store(false, Ordering::SeqCst); + + callbacks.replace_after_unregistration( + || unregistered.store(true, Ordering::SeqCst), + DropCounter { unregistered: Arc::clone(&unregistered), drops: Arc::clone(&drops) }); + assert_eq!(drops.load(Ordering::SeqCst), 1); + unregistered.store(false, Ordering::SeqCst); + + callbacks.clear_after_unregistration(|| unregistered.store(true, Ordering::SeqCst)); + assert_eq!(drops.load(Ordering::SeqCst), 2); + } +} + #[derive(Clone, PartialEq, Eq, Hash, Debug)] pub struct TrackDescription { pub id: i32, pub name: Option, } -