Streams: avoid id duplications

Use Arc<str> to share string ids as much as possible.

src/media/playback_context.rs

@@ -184,8 +184,8 @@ impl PlaybackContext {
         }
     }
 
-    pub fn select_streams(&self, stream_ids: &[String]) {
-        let stream_id_vec: Vec<&str> = stream_ids.iter().map(|id| id.as_str()).collect();
+    pub fn select_streams(&self, stream_ids: &[Arc<str>]) {
+        let stream_id_vec: Vec<&str> = stream_ids.iter().map(|id| id.as_ref()).collect();
         let select_streams_evt = gst::Event::new_select_streams(&stream_id_vec).build();
         self.decodebin.send_event(select_streams_evt);
 

src/metadata/media_info.rs

@@ -3,14 +3,15 @@ use gstreamer as gst;
 
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
+use std::sync::Arc;
 
 pub fn get_default_chapter_title() -> String {
     gettext("untitled")
 }
 
 #[derive(Clone)]
 pub struct Stream {
-    pub id: String,
+    pub id: Arc<str>,
     pub codec_printable: String,
     pub caps: gst::Caps,
     pub tags: Option<gst::TagList>,
@@ -80,7 +81,7 @@ impl Stream {
         };
 
         Stream {
-            id: stream.get_stream_id().unwrap(),
+            id: stream.get_stream_id().unwrap().into(),
             codec_printable,
             caps,
             tags,
@@ -92,16 +93,15 @@ impl Stream {
 
 #[derive(Default)]
 pub struct Streams {
-    pub audio: HashMap<String, Stream>,
-    pub video: HashMap<String, Stream>,
-    pub text: HashMap<String, Stream>,
+    pub audio: HashMap<Arc<str>, Stream>,
+    pub video: HashMap<Arc<str>, Stream>,
+    pub text: HashMap<Arc<str>, Stream>,
 
-    // FIXME: see if a self ref Stream could be used instead
-    cur_audio_id: Option<String>,
+    cur_audio_id: Option<Arc<str>>,
     pub audio_changed: bool,
-    cur_video_id: Option<String>,
+    cur_video_id: Option<Arc<str>>,
     pub video_changed: bool,
-    cur_text_id: Option<String>,
+    cur_text_id: Option<Arc<str>>,
     pub text_changed: bool,
 }
 
@@ -110,15 +110,15 @@ impl Streams {
         let stream = Stream::new(gst_stream);
         match stream.type_ {
             gst::StreamType::AUDIO => {
-                self.cur_audio_id.get_or_insert(stream.id.clone());
+                self.cur_audio_id.get_or_insert(Arc::clone(&stream.id));
                 self.audio.insert(stream.id.clone(), stream);
             }
             gst::StreamType::VIDEO => {
-                self.cur_video_id.get_or_insert(stream.id.clone());
+                self.cur_video_id.get_or_insert(Arc::clone(&stream.id));
                 self.video.insert(stream.id.clone(), stream);
             }
             gst::StreamType::TEXT => {
-                self.cur_text_id.get_or_insert(stream.id.clone());
+                self.cur_text_id.get_or_insert(Arc::clone(&stream.id));
                 self.text.insert(stream.id.clone(), stream);
             }
             _ => panic!("MediaInfo::add_stream can't handle {:?}", stream.type_),
@@ -151,8 +151,20 @@ impl Streams {
             .map(|stream_id| &self.text[stream_id])
     }
 
+    pub fn get_audio_mut<'a, S: AsRef<str>>(&'a mut self, id: S) -> Option<&'a mut Stream> {
+        self.audio.get_mut(id.as_ref())
+    }
+
+    pub fn get_video_mut<'a, S: AsRef<str>>(&'a mut self, id: S) -> Option<&'a mut Stream> {
+        self.video.get_mut(id.as_ref())
+    }
+
+    pub fn get_text_mut<'a, S: AsRef<str>>(&'a mut self, id: S) -> Option<&'a mut Stream> {
+        self.text.get_mut(id.as_ref())
+    }
+
     // Returns the streams which changed
-    pub fn select_streams(&mut self, ids: &[String]) {
+    pub fn select_streams(&mut self, ids: &[Arc<str>]) {
         let mut is_audio_selected = false;
         let mut is_text_selected = false;
         let mut is_video_selected = false;
@@ -161,20 +173,20 @@ impl Streams {
             if self.audio.contains_key(id) {
                 is_audio_selected = true;
                 self.audio_changed = self.selected_audio()
-                    .map_or(true, |prev_stream| *id != prev_stream.id.as_str());
-                self.cur_audio_id = Some(id.to_string());
+                    .map_or(true, |prev_stream| *id != prev_stream.id);
+                self.cur_audio_id = Some(Arc::clone(id));
             } else if self.text.contains_key(id) {
                 is_text_selected = true;
                 self.text_changed = self.selected_text()
-                    .map_or(true, |prev_stream| *id != prev_stream.id.as_str());
-                self.cur_text_id = Some(id.to_string());
+                    .map_or(true, |prev_stream| *id != prev_stream.id);
+                self.cur_text_id = Some(Arc::clone(id));
             } else if self.video.contains_key(id) {
                 is_video_selected = true;
                 self.video_changed = self.selected_video()
-                    .map_or(true, |prev_stream| *id != prev_stream.id.as_str());
-                self.cur_video_id = Some(id.to_string());
+                    .map_or(true, |prev_stream| *id != prev_stream.id);
+                self.cur_video_id = Some(Arc::clone(id));
             } else {
-                panic!("MediaInfo::select_streams unknown stream id {}", id);
+                panic!("MediaInfo::select_streams unknown stream id {}", id.as_ref());
             }
         }
 

src/ui/main_controller.rs

@@ -433,7 +433,7 @@ impl MainController {
         }
     }
 
-    pub fn select_streams(&mut self, stream_ids: &[String]) {
+    pub fn select_streams(&mut self, stream_ids: &[Arc<str>]) {
         self.context.as_ref().unwrap().select_streams(stream_ids);
         // In Playing state, wait for the notification from the Context
         // Otherwise, update immediately

src/ui/streams_controller.rs

@@ -1,5 +1,6 @@
 use std::cell::RefCell;
 use std::rc::{Rc, Weak};
+use std::sync::Arc;
 
 use gettextrs::gettext;
 use glib;
@@ -59,15 +60,15 @@ macro_rules! on_stream_selected(
 pub struct StreamsController {
     video_treeview: gtk::TreeView,
     video_store: gtk::ListStore,
-    video_selected: Option<String>,
+    video_selected: Option<Arc<str>>,
 
     audio_treeview: gtk::TreeView,
     audio_store: gtk::ListStore,
-    audio_selected: Option<String>,
+    audio_selected: Option<Arc<str>>,
 
     text_treeview: gtk::TreeView,
     text_store: gtk::ListStore,
-    text_selected: Option<String>,
+    text_selected: Option<Arc<str>>,
 
     main_ctrl: Option<Weak<RefCell<MainController>>>,
 }
@@ -158,8 +159,7 @@ impl StreamsController {
                         .write()
                         .unwrap()
                         .streams
-                        .video
-                        .get_mut(&stream_id)
+                        .get_video_mut(stream_id)
                         .as_mut()
                         .unwrap()
                         .must_export = value;
@@ -178,8 +178,7 @@ impl StreamsController {
                         .write()
                         .unwrap()
                         .streams
-                        .audio
-                        .get_mut(&stream_id)
+                        .get_audio_mut(stream_id)
                         .as_mut()
                         .unwrap()
                         .must_export = value;
@@ -197,8 +196,7 @@ impl StreamsController {
                         .write()
                         .unwrap()
                         .streams
-                        .text
-                        .get_mut(&stream_id)
+                        .get_text_mut(stream_id)
                         .as_mut()
                         .unwrap()
                         .must_export = value;
@@ -224,11 +222,11 @@ impl StreamsController {
             let mut sorted_ids = info.streams
                 .video
                 .keys()
-                .map(|key| key.to_string())
-                .collect::<Vec<String>>();
+                .map(|key| Arc::clone(key))
+                .collect::<Vec<Arc<str>>>();
             sorted_ids.sort();
             for stream_id in sorted_ids {
-                let mut stream = info.streams.video.get_mut(&stream_id).unwrap();
+                let mut stream = info.streams.get_video_mut(stream_id).unwrap();
                 stream.must_export = true;
                 let iter = self.add_stream(&self.video_store, stream);
                 let caps_structure = stream.caps.get_structure(0).unwrap();
@@ -246,11 +244,11 @@ impl StreamsController {
             let mut sorted_ids = info.streams
                 .audio
                 .keys()
-                .map(|key| key.to_string())
-                .collect::<Vec<String>>();
+                .map(|key| Arc::clone(key))
+                .collect::<Vec<Arc<str>>>();
             sorted_ids.sort();
             for stream_id in sorted_ids {
-                let mut stream = info.streams.audio.get_mut(&stream_id).unwrap();
+                let mut stream = info.streams.get_audio_mut(stream_id).unwrap();
                 stream.must_export = true;
                 let iter = self.add_stream(&self.audio_store, stream);
                 let caps_structure = stream.caps.get_structure(0).unwrap();
@@ -271,11 +269,11 @@ impl StreamsController {
             let mut sorted_ids = info.streams
                 .text
                 .keys()
-                .map(|key| key.to_string())
-                .collect::<Vec<String>>();
+                .map(|key| Arc::clone(key))
+                .collect::<Vec<Arc<str>>>();
             sorted_ids.sort();
             for stream_id in sorted_ids {
-                let mut stream = info.streams.text.get_mut(&stream_id).unwrap();
+                let mut stream = info.streams.get_text_mut(stream_id).unwrap();
                 stream.must_export = true;
                 let iter = self.add_stream(&self.text_store, stream);
                 let caps_structure = stream.caps.get_structure(0).unwrap();
@@ -305,15 +303,15 @@ impl StreamsController {
     pub fn trigger_stream_selection(&self) {
         // Asynchronoulsy notify the main controller
         let main_ctrl_weak = Weak::clone(self.main_ctrl.as_ref().unwrap());
-        let mut streams: Vec<String> = Vec::new();
+        let mut streams: Vec<Arc<str>> = Vec::new();
         if let Some(stream) = self.video_selected.as_ref() {
-            streams.push(stream.clone());
+            streams.push(Arc::clone(stream));
         }
         if let Some(stream) = self.audio_selected.as_ref() {
-            streams.push(stream.clone());
+            streams.push(Arc::clone(stream));
         }
         if let Some(stream) = self.text_selected.as_ref() {
-            streams.push(stream.clone());
+            streams.push(Arc::clone(stream));
         }
         gtk::idle_add(move || {
             let main_ctrl_rc = main_ctrl_weak.upgrade().unwrap();
@@ -333,7 +331,7 @@ impl StreamsController {
         let iter = store.insert_with_values(
             None,
             &[EXPORT_FLAG_COL, STREAM_ID_COL, STREAM_ID_DISPLAY_COL],
-            &[&true, &stream.id, &stream_id_display],
+            &[&true, &stream.id.as_ref(), &stream_id_display],
         );
 
         if let Some(ref tags) = stream.tags {
@@ -360,11 +358,12 @@ impl StreamsController {
         iter
     }
 
-    fn get_stream_at(&self, store: &gtk::ListStore, iter: &gtk::TreeIter) -> String {
+    fn get_stream_at(&self, store: &gtk::ListStore, iter: &gtk::TreeIter) -> Arc<str> {
         store
             .get_value(iter, STREAM_ID_COL as i32)
             .get::<String>()
             .unwrap()
+            .into()
     }
 
     fn init_treeviews(&self, this_rc: &Rc<RefCell<Self>>) {