Better control over the playlist

Use signals ('play-index' for example) to communicate between the main
window and the playlist.

Save and Load playlists and Add track methods are reduced in
complexity.

Signed-off-by: Manuel Kaufmann <humitos@gmail.com>
Reviewed-by: Gonzalo Odiard <gonzalo@laptop.org>

4 files changed, 186 insertions, 248 deletions

diff --git a/activity.py b/activity.py
index e163a6c..9b7472b 100644
--- a/activity.py
+++ b/activity.py
@@ -60,6 +60,10 @@ PLAYLIST_WIDTH_PROP = 1.0 / 3
 class JukeboxActivity(activity.Activity):
     UPDATE_INTERVAL = 500
 
+    __gsignals__ = {
+        'no-stream': (GObject.SignalFlags.RUN_FIRST, None, []),
+        }
+
     def __init__(self, handle):
         activity.Activity.__init__(self, handle)
         self._object_id = handle.object_id
@@ -123,15 +127,6 @@ class JukeboxActivity(activity.Activity):
         self.player = None
         self.uri = None
 
-        # {'url': 'file://.../media.ogg', 'title': 'My song', object_id: '..'}
-        self.playlist = []
-
-        self.jobjectlist = []
-        self.playpath = None
-        self.currentplaying = None
-        self.playflag = False
-        self._not_found_files = 0
-
         # README: I changed this because I was getting an error when I
         # tried to modify self.bin with something different than
         # Gtk.Bin
@@ -142,10 +137,11 @@ class JukeboxActivity(activity.Activity):
         self.canvas = Gtk.Box(orientation=Gtk.Orientation.HORIZONTAL)
         self._alert = None
 
-        self.playlist_widget = PlayList(self.play)
-        self.playlist_widget.update(self.playlist)
+        self.playlist_widget = PlayList()
+        self.playlist_widget.connect('play-index', self.__play_index_cb)
         self.playlist_widget.show()
         self.canvas.pack_start(self.playlist_widget, False, True, 0)
+
         self._empty_widget = Gtk.Label(label="")
         self._empty_widget.show()
         self.videowidget = VideoWidget()
@@ -154,16 +150,6 @@ class JukeboxActivity(activity.Activity):
         self.show_all()
         self.canvas.connect('size-allocate', self.__size_allocate_cb)
 
-        #From ImageViewer Activity
-        self._want_document = True
-        if self._object_id is None:
-            self._show_object_picker = GObject.timeout_add(1000, \
-            self._show_picker_cb)
-
-        if handle.uri:
-            self.uri = handle.uri
-            GObject.idle_add(self._start, self.uri, handle.title)
-
         # Create the player just once
         logging.debug('Instantiating GstPlayer')
         self.player = GstPlayer(self.videowidget)
@@ -219,17 +205,6 @@ class JukeboxActivity(activity.Activity):
         playlist_width = int(canvas_size.width * PLAYLIST_WIDTH_PROP)
         self.playlist_widget.set_size_request(playlist_width, 0)
 
-    def open_button_clicked_cb(self, widget):
-        """ To open the dialog to select a new file"""
-        #self.player.seek(0L)
-        #self.player.stop()
-        #self.playlist = []
-        #self.playpath = None
-        #self.currentplaying = None
-        #self.playflag = False
-        self._want_document = True
-        self._show_object_picker = GObject.timeout_add(1, self._show_picker_cb)
-
     def _key_press_event_cb(self, widget, event):
         keyname = Gdk.keyval_name(event.keyval)
         logging.info("Keyname Press: %s, time: %s", keyname, event.time)
@@ -241,59 +216,50 @@ class JukeboxActivity(activity.Activity):
             return True
 
     def check_if_next_prev(self):
-        if self.currentplaying == 0:
+        current_playing = self.playlist_widget._current_playing
+        if current_playing == 0:
             self.control.prev_button.set_sensitive(False)
         else:
             self.control.prev_button.set_sensitive(True)
-        if self.currentplaying == len(self.playlist) - 1:
+        if current_playing == len(self.playlist_widget._items) - 1:
             self.control.next_button.set_sensitive(False)
         else:
             self.control.next_button.set_sensitive(True)
 
     def songchange(self, direction):
-        #if self.playflag:
-        #    self.playflag = False
-        #    return
-        self.player.seek(0L)
-        if direction == "prev" and self.currentplaying > 0:
-            self.play(self.currentplaying - 1)
-            logging.info("prev: " + self.playlist[self.currentplaying]['url'])
-            #self.playflag = True
-        elif direction == "next" and \
-                self.currentplaying < len(self.playlist) - 1:
-            self.play(self.currentplaying + 1)
-            logging.info("next: " + self.playlist[self.currentplaying]['url'])
-            #self.playflag = True
+        current_playing = self.playlist_widget._current_playing
+        if direction == 'prev' and current_playing > 0:
+            self.play_index(current_playing - 1)
+        elif direction == 'next' and \
+                current_playing < len(self.playlist_widget._items) - 1:
+            self.play_index(current_playing + 1)
+
         else:
-            self.play_toggled()
-            self.player.stop()
             self._switch_canvas(show_video=False)
-            self.player.set_uri(None)
-            self.check_if_next_prev()
-
-    def play(self, media_index):
-        self._switch_canvas(show_video=True)
-        self.currentplaying = media_index
-        url = self.playlist[self.currentplaying]['url']
-        error = None
-        if url.startswith('journal://'):
-            try:
-                jobject = datastore.get(url[len("journal://"):])
-                url = 'file://' + jobject.file_path
-            except:
-                path = url[len("journal://"):]
-                error = _('The file %s was not found') % path
+            self.emit('no-stream')
 
+    def play_index(self, index):
+        # README: this line is no more necessary because of the
+        # .playing_video() method
+        # self._switch_canvas(show_video=True)
+        self.playlist_widget._current_playing = index
+
+        path = self.playlist_widget._items[index]['path']
         self.check_if_next_prev()
 
-        if error is None:
-            self.player.set_uri(url)
-            self.player.play()
-        else:
-            self.control.set_disabled()
-            self._show_error_alert(error)
+        self.player.set_uri(path)
+        self.player.play()
+
+    def __play_index_cb(self, widget, index, path):
+        # README: this line is no more necessary because of the
+        # .playing_video() method
+        # self._switch_canvas(show_video=True)
+        self.playlist_widget._current_playing = index
+
+        self.check_if_next_prev()
 
-        self.playlist_widget.set_cursor(self.currentplaying)
+        self.player.set_uri(path)
+        self.player.play()
 
     def _player_eos_cb(self, widget):
         self.songchange('next')
@@ -310,12 +276,12 @@ class JukeboxActivity(activity.Activity):
     def _mount_added_cb(self, volume_monitor, device):
         self.view_area.set_current_page(0)
         self.remove_alert(self._alert)
-        self.playlist_widget.update(self.playlist)
+        self.playlist_widget.update()
 
     def _mount_removed_cb(self, volume_monitor, device):
         self.view_area.set_current_page(0)
         self.remove_alert(self._alert)
-        self.playlist_widget.update(self.playlist)
+        self.playlist_widget.update()
 
     def _show_missing_tracks_alert(self, nro):
         self._alert = Alert()
@@ -388,34 +354,6 @@ class JukeboxActivity(activity.Activity):
         logging.debug("shared start")
         pass
 
-    def _show_picker_cb(self):
-        #From ImageViewer Activity
-        if not self._want_document:
-            return
-
-        # README: some arguments are deprecated so I avoid them
-
-        # chooser = ObjectChooser(_('Choose document'), self,
-        #     Gtk.DialogFlags.MODAL |
-        #     Gtk.DialogFlags.DESTROY_WITH_PARENT,
-        #     what_filter=mime.GENERIC_TYPE_AUDIO)
-
-        chooser = ObjectChooser(self, what_filter=mime.GENERIC_TYPE_AUDIO)
-
-        try:
-            result = chooser.run()
-            if result == Gtk.ResponseType.ACCEPT:
-                jobject = chooser.get_selected_object()
-                if jobject and jobject.file_path:
-                    logging.error('Adding %s', jobject.file_path)
-                    title = jobject.metadata.get('title', None)
-                    self._load_file(jobject.file_path, title,
-                            jobject.object_id)
-        finally:
-            #chooser.destroy()
-            #del chooser
-            pass
-
     def can_close(self):
         # We need to put the Gst.State in NULL so gstreamer can
         # cleanup the pipeline
@@ -425,48 +363,9 @@ class JukeboxActivity(activity.Activity):
     def read_file(self, file_path):
         """Load a file from the datastore on activity start."""
         logging.debug('JukeBoxAtivity.read_file: %s', file_path)
-        title = self.metadata.get('title', None)
-        self._load_file(file_path, title, self._object_id)
-
-    def _load_file(self, file_path, title, object_id):
-        self.uri = os.path.abspath(file_path)
-        if os.path.islink(self.uri):
-            self.uri = os.path.realpath(self.uri)
-        mimetype = mime.get_for_file('file://' + file_path)
-        logging.error('read_file mime %s', mimetype)
-        if mimetype == 'audio/x-mpegurl':
-            # is a M3U playlist:
-            for uri in self._read_m3u_playlist(file_path):
-                if not self.playlist_widget.check_available_media(uri['url']):
-                    self._not_found_files += 1
-
-                GObject.idle_add(self._start, uri['url'], uri['title'],
-                        uri['object_id'])
-        else:
-            # is another media file:
-            GObject.idle_add(self._start, self.uri, title, object_id)
-
-        if self._not_found_files > 0:
-            self._show_missing_tracks_alert(self._not_found_files)
-
-    def _create_playlist_jobject(self):
-        """Create an object in the Journal to store the playlist.
 
-        This is needed if the activity was not started from a playlist
-        or from scratch.
-
-        """
-        jobject = datastore.create()
-        jobject.metadata['mime_type'] = "audio/x-mpegurl"
-        jobject.metadata['title'] = _('Jukebox playlist')
-
-        temp_path = os.path.join(activity.get_activity_root(),
-                                 'instance')
-        if not os.path.exists(temp_path):
-            os.makedirs(temp_path)
-
-        jobject.file_path = tempfile.mkstemp(dir=temp_path)[1]
-        self._playlist_jobject = jobject
+        title = self.metadata['title']
+        self.playlist_widget.load_file(file_path, title)
 
     def write_file(self, file_path):
 
@@ -476,10 +375,11 @@ class JukeboxActivity(activity.Activity):
             It is saved in audio/x-mpegurl format.
 
             """
+
             list_file = open(file_path, 'w')
-            for uri in self.playlist:
-                list_file.write('#EXTINF: %s\n' % uri['title'])
-                list_file.write('%s\n' % uri['url'])
+            for uri in self.playlist_widget._items:
+                list_file.write('#EXTINF:%s\n' % uri['title'])
+                list_file.write('%s\n' % uri['path'])
             list_file.close()
 
         if not self.metadata['mime_type']:
@@ -490,86 +390,19 @@ class JukeboxActivity(activity.Activity):
 
         else:
             if self._playlist_jobject is None:
-                self._create_playlist_jobject()
+                self._playlist_jobject = self.playlist_widget.create_playlist_jobject()
 
             # Add the playlist to the playlist jobject description.
             # This is only done if the activity was not started from a
             # playlist or from scratch:
             description = ''
-            for uri in self.playlist:
+            for uri in self.playlist_widget._items:
                 description += '%s\n' % uri['title']
             self._playlist_jobject.metadata['description'] = description
 
             write_playlist_to_file(self._playlist_jobject.file_path)
             datastore.write(self._playlist_jobject)
 
-    def _read_m3u_playlist(self, file_path):
-        urls = []
-        title = ''
-        for line in open(file_path).readlines():
-            line = line.strip()
-            if line != '':
-                if line.startswith('#EXTINF:'):
-                    # line with data
-                    #EXTINF: title
-                    title = line[len('#EXTINF:'):]
-                else:
-                    uri = {}
-                    uri['url'] = line.strip()
-                    uri['title'] = title
-                    if uri['url'].startswith('journal://'):
-                        uri['object_id'] = uri['url'][len('journal://'):]
-                    else:
-                        uri['object_id'] = None
-                    urls.append(uri)
-                    title = ''
-        return urls
-
-    def _start(self, uri=None, title=None, object_id=None):
-        self._want_document = False
-        self.playpath = os.path.dirname(uri)
-        if not uri:
-            return False
-
-        if title is not None:
-            title = title.strip()
-        if object_id is not None:
-            self.playlist.append({'url': 'journal://' + object_id,
-                    'title': title})
-        else:
-            if uri.startswith("file://"):
-                self.playlist.append({'url': uri, 'title': title})
-            else:
-                uri = "file://" + urllib.quote(os.path.abspath(uri))
-                self.playlist.append({'url': uri, 'title': title})
-
-        self.playlist_widget.update(self.playlist)
-
-        try:
-            if self.currentplaying is None:
-                logging.info("Playing: " + self.playlist[0]['url'])
-                url = self.playlist[0]['url']
-                if url.startswith('journal://'):
-                    jobject = datastore.get(url[len("journal://"):])
-                    url = 'file://' + jobject.file_path
-
-                self.player.set_uri(url)
-                self.player.play()
-                self.currentplaying = 0
-                self.play_toggled()
-                self.show_all()
-            else:
-                pass
-                #self.player.seek(0L)
-                #self.player.stop()
-                #self.currentplaying += 1
-                #self.player.set_uri(self.playlist[self.currentplaying])
-                #self.play_toggled()
-        except:
-            pass
-        self.check_if_next_prev()
-        return False
-
     def play_toggled(self):
         self.control.set_enabled()
 
@@ -657,9 +490,6 @@ class JukeboxActivity(activity.Activity):
 
         return True
 
-    def _erase_playlist_entry_clicked_cb(self, widget):
-        self.playlist_widget.delete_selected_items()
-
     def __go_fullscreen_cb(self, toolbar):
         self.fullscreen()
 
diff --git a/controls.py b/controls.py
index b84a55a..09b5325 100644
--- a/controls.py
+++ b/controls.py
@@ -22,7 +22,9 @@ from gi.repository import GObject
 
 from gettext import gettext as _
 
+from sugar3 import mime
 from sugar3.graphics.toolbutton import ToolButton
+from sugar3.graphics.objectchooser import ObjectChooser
 
 
 class Controls(GObject.GObject):
@@ -38,13 +40,13 @@ class Controls(GObject.GObject):
         self.open_button = ToolButton('list-add')
         self.open_button.set_tooltip(_('Add track'))
         self.open_button.show()
-        self.open_button.connect('clicked', jukebox.open_button_clicked_cb)
+        self.open_button.connect('clicked', self.__open_button_clicked_cb)
         self.toolbar.insert(self.open_button, -1)
 
         erase_playlist_entry_btn = ToolButton(icon_name='list-remove')
         erase_playlist_entry_btn.set_tooltip(_('Remove track'))
         erase_playlist_entry_btn.connect('clicked',
-                 jukebox._erase_playlist_entry_clicked_cb)
+                 self.__erase_playlist_entry_clicked_cb)
         self.toolbar.insert(erase_playlist_entry_btn, -1)
 
         spacer = Gtk.SeparatorToolItem()
@@ -108,6 +110,29 @@ class Controls(GObject.GObject):
         total_time.show()
         toolbar.insert(total_time, -1)
 
+    def __open_button_clicked_cb(self, widget):
+        self.__show_picker_cb()
+
+    def __erase_playlist_entry_clicked_cb(self, widget):
+        self.jukebox.playlist_widget.delete_selected_items()
+
+    def __show_picker_cb(self):
+        jobject = None
+        chooser = ObjectChooser(self.jukebox,
+                                what_filter=mime.GENERIC_TYPE_AUDIO)
+
+        try:
+            result = chooser.run()
+            if result == Gtk.ResponseType.ACCEPT:
+                jobject = chooser.get_selected_object()
+                if jobject and jobject.file_path:
+                    logging.info('Adding %s', jobject.file_path)
+                    self.jukebox.playlist_widget.load_file(jobject)
+                    self.jukebox.check_if_next_prev()
+        finally:
+            if jobject is not None:
+                jobject.destroy()
+
     def prev_button_clicked_cb(self, widget):
         self.jukebox.songchange('prev')
 
diff --git a/player.py b/player.py
index 5e5305b..39e68f5 100644
--- a/player.py
+++ b/player.py
@@ -85,7 +85,9 @@ class GstPlayer(GObject.GObject):
 
     def set_uri(self, uri):
         self.pipeline.set_state(Gst.State.READY)
-        logging.debug('### Setting URI: %s', uri)
+        # gstreamer needs the 'file://' prefix
+        uri = 'file://' + uri
+        logging.debug('URI: %s', uri)
         self.player.set_property('uri', uri)
 
     def query_position(self):
diff --git a/playlist.py b/playlist.py
index 4563d47..653cdbc 100644
--- a/playlist.py
+++ b/playlist.py
@@ -13,31 +13,38 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 # USA
 
-import logging
 import os
+import logging
+import tempfile
 from gettext import gettext as _
 
-import gi
-gi.require_version('Gtk', '3.0')
-
 from gi.repository import GObject
 from gi.repository import Gtk
 from gi.repository import Pango
 
+from sugar3 import mime
+from sugar3.datastore import datastore
+from sugar3.activity import activity
 from sugar3.graphics.icon import CellRendererIcon
 
 
-COLUMNS_NAME = ('index', 'media', 'available')
+COLUMNS_NAME = ('index', 'title', 'available')
 COLUMNS = dict((name, i) for i, name in enumerate(COLUMNS_NAME))
 
 
 class PlayList(Gtk.ScrolledWindow):
-    def __init__(self, play_callback):
-        self._playlist = None
-        self._play_callback = play_callback
+
+    __gsignals__ = {
+        'play-index': (GObject.SignalFlags.RUN_FIRST, None, [int, str]),
+        }
+
+    def __init__(self):
+        self._not_found_files = 0
+        self._current_playing = None
+        self._items = []
 
         GObject.GObject.__init__(self, hadjustment=None,
-                                    vadjustment=None)
+                                 vadjustment=None)
         self.set_policy(Gtk.PolicyType.NEVER, Gtk.PolicyType.AUTOMATIC)
         self.listview = Gtk.TreeView()
         self.treemodel = Gtk.ListStore(int, object, bool)
@@ -63,7 +70,7 @@ class PlayList(Gtk.ScrolledWindow):
 
         renderer_title = Gtk.CellRendererText()
         renderer_title.set_property('ellipsize', Pango.EllipsizeMode.END)
-        treecol_title = Gtk.TreeViewColumn(_('Play List'))
+        treecol_title = Gtk.TreeViewColumn(_('Track'))
         treecol_title.pack_start(renderer_title, True)
         treecol_title.set_cell_data_func(renderer_title, self._set_title)
         self.listview.append_column(treecol_title)
@@ -79,18 +86,23 @@ class PlayList(Gtk.ScrolledWindow):
         model = treeview.get_model()
 
         treeiter = model.get_iter(path)
-        media_idx = model.get_value(treeiter, COLUMNS['index'])
-        self._play_callback(media_idx)
+        index = model.get_value(treeiter, COLUMNS['index'])
+        # TODO: put the path inside the ListStore
+        path = self._items[index]['path']
+        # TODO: check if the stream is available before emitting the signal
+        self._current_playing = index
+        self.set_cursor(index)
+        self.emit('play-index', index, path)
 
     def _set_number(self, column, cell, model, it, data):
         idx = model.get_value(it, COLUMNS['index'])
         cell.set_property('text', idx + 1)
 
     def _set_title(self, column, cell, model, it, data):
-        playlist_item = model.get_value(it, COLUMNS['media'])
+        title = model.get_value(it, COLUMNS['title'])
         available = model.get_value(it, COLUMNS['available'])
 
-        cell.set_property('text', playlist_item['title'])
+        cell.set_property('text', title)
         sensitive = True
         if not available:
             sensitive = False
@@ -100,16 +112,6 @@ class PlayList(Gtk.ScrolledWindow):
         available = model.get_value(it, COLUMNS['available'])
         cell.set_property('visible', not available)
 
-    def update(self, playlist):
-        self.treemodel.clear()
-        self._playlist = playlist
-        pl = list(enumerate(playlist))
-        for i, media in pl:
-            available = self.check_available_media(media['url'])
-            media['available'] = available
-            self.treemodel.append((i, media, available))
-        #self.set_cursor(0)
-
     def set_cursor(self, index):
         self.listview.set_cursor((index,))
 
@@ -118,12 +120,10 @@ class PlayList(Gtk.ScrolledWindow):
         sel_model, sel_rows = self.listview.get_selection().get_selected_rows()
         for row in sel_rows:
             index = sel_model.get_value(sel_model.get_iter(row), 0)
-            self._playlist.pop(index)
+            self._items.pop(index)
             self.treemodel.remove(self.treemodel.get_iter(row))
-        self.update(self._playlist)
 
-    def check_available_media(self, uri):
-        path = uri.replace('journal://', '').replace('file://', '')
+    def check_available_media(self, path):
         if os.path.exists(path):
             return True
         else:
@@ -135,3 +135,84 @@ class PlayList(Gtk.ScrolledWindow):
             if not track['available']:
                 missing_tracks.append(track)
         return missing_tracks
+
+    def _load_m3u_playlist(self, file_path):
+        for uri in self._read_m3u_playlist(file_path):
+            if not self.check_available_media(uri['path']):
+                self._not_found_files += 1
+
+            self._add_track(uri['path'], uri['title'])
+
+    def _load_stream(self, file_path, title=None):
+        # TODO: read id3 here
+        if os.path.islink(file_path):
+            file_path = os.path.realpath(file_path)
+        self._add_track(file_path, title)
+
+    def load_file(self, jobject, title=None):
+        logging.debug('#### jobject: %s', type(jobject))
+        if isinstance(jobject, datastore.RawObject) or \
+           isinstance(jobject, datastore.DSObject):
+            file_path = jobject.file_path
+            title = jobject.metadata['title']
+        else:
+            file_path = jobject
+
+        mimetype = mime.get_for_file('file://' + file_path)
+        logging.info('read_file mime %s', mimetype)
+        if mimetype == 'audio/x-mpegurl':
+            # is a M3U playlist:
+            self._load_m3u_playlist(file_path)
+        else:
+            # is not a M3U playlist
+            self._load_stream(file_path, title)
+
+    def update(self):
+        for tree_item, playlist_item in zip(self.treemodel, self._items):
+            tree_item[2] = self.check_available_media(playlist_item['path'])
+
+    def _add_track(self, file_path, title):
+        available = self.check_available_media(file_path)
+        item = {'path': file_path,
+                'title': title,
+                'available': available}
+        self._items.append(item)
+        index = len(self._items) - 1
+        self.treemodel.append((index, item['title'], available))
+
+    def _read_m3u_playlist(self, file_path):
+        urls = []
+        title = ''
+        for line in open(file_path).readlines():
+            line = line.strip()
+            if line != '':
+                if line.startswith('#EXTINF:'):
+                    # line with data
+                    # EXTINF: title
+                    title = line[len('#EXTINF:'):]
+                else:
+                    uri = {}
+                    uri['path'] = line.strip()
+                    uri['title'] = title
+                    urls.append(uri)
+                    title = ''
+        return urls
+
+    def create_playlist_jobject(self):
+        """Create an object in the Journal to store the playlist.
+
+        This is needed if the activity was not started from a playlist
+        or from scratch.
+        """
+
+        jobject = datastore.create()
+        jobject.metadata['mime_type'] = "audio/x-mpegurl"
+        jobject.metadata['title'] = _('Jukebox playlist')
+
+        temp_path = os.path.join(activity.get_activity_root(),
+                                 'instance')
+        if not os.path.exists(temp_path):
+            os.makedirs(temp_path)
+
+        jobject.file_path = tempfile.mkstemp(dir=temp_path)[1]
+        return jobject