Merge lp:~agronick/vocal/vocal into lp:vocal

[Kyle]

Vocal was crashing with a segmentation fault when playing certain videos. When I looked at the URL I found that it was doing a 302 redirect and setting the Location header. In Player.vala Gst.PbUtils.DiscovererInfo.get_video_streams() did not know how to handle this and would return an empty list. When it was cast to DiscovererVideoInfo a segmentation fault is caused.

Many podcasting services use redirects for analytics and tracking. All of the videos in this RSS feed will crash Vocal unless my code is used: http://feeds2.feedburner.com/AllJupiterVideos

The new code also prevents Vocal from cashing if get_video_streams() returns an empty list for some other reason.

[lp:~agronick/vocal/vocal]

296. By Kyle on 2015-06-06

Fixed segfault with certain podcasts

[Kyle (agronick)]

I also fixed the bug that was causing the segfault with certain feeds. This was from accessing an array out of bounds in Eppisode.vala if the dates are not formatted correctly. After checking the length of the array it was displaying "(null)" where the dates are set to appear on the sidepane. I made it so it shows an empty string instead.

[Kyle (agronick)]

With regard to the first bug, some of the URLs in the feed may work. If you use this URL:
http://www.podtrac.com/pts/redirect.mp4/201406.jb-dl.cdn.scaleengine.net/unfilter/2015/unfilter-0144.mp4

It is one that will definitely crash with the current code and work with the new code.

[lp:~agronick/vocal/vocal]

297. By Kyle on 2015-06-06

Merging changes

298. By Kyle on 2015-06-08

Turns out redirects were not the issue with the origional bug. The timeouts were too small. Removed redirect handling code. Kept code preventing segmentation faults. Increased timeouts.

[Kyle (agronick)]

Turns out redirects were not the issue with the original bug. The timeouts were too small. Removed redirect handling code. Kept code preventing segmentation faults. CMakeLists.txt can be kept in it's original state but we probably want to remove the -g option for release builds. The first fix is in Player.vala and the second one is in Episode.vala. Email me for any questions: <email address hidden>

[lp:~agronick/vocal/vocal]

299. By Kyle on 2015-06-08

New seek method makes the player a lot more responsive.

Unmerged revisions

299. By Kyle on 2015-06-08

New seek method makes the player a lot more responsive.

298. By Kyle on 2015-06-08

Turns out redirects were not the issue with the origional bug. The timeouts were too small. Removed redirect handling code. Kept code preventing segmentation faults. Increased timeouts.

297. By Kyle on 2015-06-06

Merging changes

296. By Kyle on 2015-06-06

Fixed segfault with certain podcasts

295. By Kyle on 2015-06-06

Fixed bug with redirecting URLs

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2015-04-05 16:46:38 +0000
+++ CMakeLists.txt	2015-06-08 19:25:26 +0000
@@ -15,7 +15,7 @@
 list (APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
 
 file(GLOB_RECURSE sources src/*.vala)
-
+ 
 # Some configuration
 configure_file (${CMAKE_SOURCE_DIR}/src/config.vala.cmake ${CMAKE_SOURCE_DIR}/src/config.vala)
 
@@ -35,6 +35,7 @@
     --disable-warnings
     --target-glib=2.32
     --thread
+    --save-temps  
     -g
 )
 
 
=== modified file 'src/Objects/Episode.vala'
--- src/Objects/Episode.vala	2015-04-17 01:14:15 +0000
+++ src/Objects/Episode.vala	2015-06-08 19:25:26 +0000
@@ -122,15 +122,19 @@
                 offset = -2;
             }
 
-            
-        
+            if(fields.length < 7)
+                return; 
+
             // Since the pubdate is a standard, we can hard-code the values where each item is located
-            
             int day = int.parse(fields[2 + offset]);
             int year = int.parse(fields[4 + offset]);
             int hour = int.parse(fields[5 + offset]);
             int minute = int.parse(fields[6 + offset]);
             int seconds = int.parse(fields[7 + offset]);
+
+            if(day == 0 || year == 0)
+                return;
+
                 
             
             // Determine the month number from the string as read from the pubdate
 
=== modified file 'src/Utils/Player.vala'
--- src/Utils/Player.vala	2015-04-04 21:51:53 +0000
+++ src/Utils/Player.vala	2015-06-08 19:25:26 +0000
@@ -20,18 +20,18 @@
 using Clutter;
 using GLib;
 using Gst;
-using Gst.PbUtils;
+using Gst.PbUtils; 
 
 namespace Vocal {
     class Player : Actor {
 
-    	private static Player? player = null;
+        private static Player? player = null;
         public static Player? get_default (string[] args) {
             if (player == null)
                 player = new Player(args);
             return player;
         }
-    
+
         public signal void state_changed(Gst.State new_state);
         public signal void stream_ended();
         public signal void additional_plugins_required(Gst.Message message);
@@ -43,20 +43,20 @@
 
         private MainLoop loop = new MainLoop ();
         public signal void new_position_available();
-        private string tag_string;       
-        
+        private string tag_string;
+
         public bool is_currently_playing;
-        
+
         public Gst.State current_state;
-        
+
         public Episode current_episode;
-        
+
         private Player(string[]? args) {
 
-        	/*	NOTE:
-				This code is heavily influenced by the Audience source code, written by Tom Beckmann <[e-mail]>
+            /*  NOTE:
+                This code is heavily influenced by the Audience source code, written by Tom Beckmann <[e-mail]>
                 and Corentin Noël <[e-mail]>, available at https://launchpad.net/audience.
-        	 */
+             */
 
             // Set up the Clutter actor
             video = new Clutter.Texture ();
@@ -77,7 +77,7 @@
             current_episode = null;
 
             is_currently_playing = false;
-            
+
             // Check every half-second if the current media is playing, and if it is
             // send a signal that there is a new position available
             GLib.Timeout.add(500, () => {
@@ -85,14 +85,14 @@
                     new_position_available();
                 return true;
             });
-                   
+
         }
-        
+
         /*
          * Bus callback
          */
         private bool bus_callback (Gst.Bus bus, Gst.Message message) {
-        
+
             switch (message.type) {
                 case MessageType.ERROR:
                     GLib.Error err;
@@ -115,11 +115,11 @@
                     Gst.State oldstate;
                     Gst.State newstate;
                     Gst.State pending;
-                    
-                    
+
+
                     message.parse_state_changed (out oldstate, out newstate,
                                                  out pending);
-                                                 
+
                     current_state = newstate;
                     state_changed(newstate);
 
@@ -135,30 +135,30 @@
 
             return true;
         }
-        
+
         /*
-		 * Reconfigures the video (width, positions, etc.) as needed
-		 */
+         * Reconfigures the video (width, positions, etc.) as needed
+         */
         public void configure_video() {
 
-        	// Make sure the video has both a known width and height at this point
-        	if(video_width > 0 && video_height > 0) {
-
-	            // Get the stage
-            	var stage = get_stage ();
-
-            	// Calculate the videos aspect ratio
-
-		        var aspect = (float)stage.width / (float)video_width < (float)stage.height / (float)video_height ?
-                	(float)stage.width / (float)video_width : (float)stage.height / (float)video_height;
-
-	            video.width  = video_width * aspect;
-	            video.height = video_height * aspect;
-	            video.x = (stage.width  - video.width)  / 2;
-	            video.y = (stage.height - video.height) / 2;
+            // Make sure the video has both a known width and height at this point
+            if(video_width > 0 && video_height > 0) {
+
+                // Get the stage
+                var stage = get_stage ();
+
+                // Calculate the videos aspect ratio
+
+                var aspect = (float)stage.width / (float)video_width < (float)stage.height / (float)video_height ?
+                    (float)stage.width / (float)video_width : (float)stage.height / (float)video_height;
+
+                video.width  = video_width * aspect;
+                video.height = video_height * aspect;
+                video.x = (stage.width  - video.width)  / 2;
+                video.y = (stage.height - video.height) / 2;
             }
         }
-        
+
         /*
          * Iterates through a list of tags and sets matching properties
          */
@@ -171,28 +171,28 @@
                 break;
             }
         }
-        
+
         /*
          *  Returns the total duration of the currently playing media
          */
         public int64 get_duration () {
             int64 rv = (int64)0;
             Gst.Format f = Gst.Format.TIME;
-            
+
             playbin.query_duration (f, out rv);
-            
+
             return rv;
         }
-        
+
         /*
          * Returns the current position of the media
          */
         public int64 get_position () {
             int64 rv = (int64)0;
             Gst.Format f = Gst.Format.TIME;
-            
+
             playbin.query_position (f, out rv);
-            
+
             return rv;
         }
 /*
@@ -202,21 +202,21 @@
         }
 
         public double get_volume () {
-	        var val = GLib.Value (typeof(double));
-	        playbin.get_property ("volume", ref val);
-	        return (double)val;
-	    }
+            var val = GLib.Value (typeof(double));
+            playbin.get_property ("volume", ref val);
+            return (double)val;
+        }
 */
-	    
-	    /*
+
+        /*
          * Sets the current state to pause
          */
         public void pause () {
             set_state (Gst.State.PAUSED);
             is_currently_playing = false;
         }
-	    
-	    /*
+
+        /*
          * Sets the current state to playing
          */
         public void play () {
@@ -225,7 +225,7 @@
 
             is_currently_playing = true;
         }
-        
+
         /*
          * Seeks backward in the currently playing media n number of seconds
          */
@@ -235,13 +235,13 @@
             int64 current = this.get_position();
             int64 new_position = current - mil;
             this.set_position(new_position);
-            
+
             new_position_available();
-            
+
             // If it was paused before, pause it again
             set_state(previous);
         }
-        
+
         /*
          * Seeks forward in the currently playing media n number of seconds
          */
@@ -251,13 +251,13 @@
             int64 current = this.get_position();
             int64 new_position = current + mil;
             this.set_position(new_position);
-            
+
             new_position_available();
-            
+
             // If it was paused before, pause it again
             set_state(previous);
         }
-        
+
         /*
          * Sets the episode that is currently being played
          */
@@ -266,38 +266,44 @@
             // Set the previous state to ready
             set_state(Gst.State.PAUSED);
             set_state(Gst.State.READY);
-            
+
             this.current_episode = episode;
-            
+
             // Set playbin to null
             set_state(Gst.State.NULL);
-            
+
             // Set the URI
             playbin.uri = episode.playback_uri;
             info("Setting URI: %s".printf(episode.playback_uri));
-            
+
+            Gst.Bus bus = playbin.get_bus ();
+            bus.add_watch (0, bus_callback); 
+
             // If it's a video podcast, get the width and height and configure that information
             if(episode.parent.content_type == MediaType.VIDEO) {
-	            try {
-		            var info = new Gst.PbUtils.Discoverer (10 * Gst.SECOND).discover_uri (playbin.uri);
-		            var video = info.get_video_streams ();
-		            if (video.data != null) {
-		                var video_info = (Gst.PbUtils.DiscovererVideoInfo)video.data;
-		                video_width = video_info.get_width ();
-		                video_height = video_info.get_height ();
-
-		                configure_video();
-
-		            }
-	            }catch(Error e) {
-	            	warning(e.message);
-	            }
-            }
-    
-            Gst.Bus bus = playbin.get_bus ();
-            bus.add_watch (0, bus_callback);
+
+               var info = new Gst.PbUtils.Discoverer (50 * Gst.SECOND).discover_uri (episode.playback_uri);
+               var video = info.get_video_streams ();
+
+                try {
+ 
+                    video.foreach ((entry) => {
+                        DiscovererVideoInfo video_info = (DiscovererVideoInfo)entry;
+                        video_width = video_info.get_width ();
+                        video_height = video_info.get_height (); 
+                    });
+
+                    configure_video();
+
+                }catch(Error e) {
+                    warning(e.message);
+                }
+            } 
+
+
         }
-        
+ 
+
         /*
          * Change the playback rate
          */
@@ -307,35 +313,20 @@
             Gst.Format.TIME, Gst.SeekFlags.SKIP,
             Gst.SeekType.NONE, pos,
             Gst.SeekType.NONE, get_duration ());
-        } 
-        
+        }
+
         /*
          *  Sets the currently playing media position
          */
-        public void set_position (int64 pos) {
-
-        	// Slowly dial the volume back (should help avoid the crappy choppy sound when streaming)
-        	for(double i = 0.1; i <= 1; i += 0.1) {
-        		set_volume(1.0 - i);
-        		Thread.usleep(50000);
-        	}
-            
-            playbin.seek (1.0,
-            Gst.Format.TIME, Gst.SeekFlags.FLUSH,
-            Gst.SeekType.SET, pos,
-            Gst.SeekType.NONE, get_duration ());
-
-                       
-            // Let this thread sleep for .7 seconds to let GStreamer get caught up 
-            Thread.usleep(700000);
-
-            // Turn the volume back up
-            for(double j = 0.1; j <= 1; j += 0.1) {
-                set_volume(j);
-                Thread.usleep(10000);
-            }
+        public void set_position (int64 pos) { 
+ 
+            playbin.seek_simple (Gst.Format.TIME, 
+                                 Gst.SeekFlags.FLUSH | 
+                                 Gst.SeekFlags.KEY_UNIT, pos);
+
+ 
         }
-        
+
         /*
          * Set the player to a designated state
          */
@@ -348,11 +339,11 @@
             playbin.set_state (s);
         }
 
-		/*
-		 * Sets the current volume
-		 */
-	    public void set_volume (double val) {
-	        playbin.set_property ("volume", val);
-	    }
+        /*
+         * Sets the current volume
+         */
+        public void set_volume (double val) {
+            playbin.set_property ("volume", val);
+        }
     }
 }
 
=== modified file 'src/Widgets/EpisodeDetailBox.vala'
--- src/Widgets/EpisodeDetailBox.vala	2015-04-04 19:32:30 +0000
+++ src/Widgets/EpisodeDetailBox.vala	2015-06-08 19:25:26 +0000
@@ -150,7 +150,8 @@
             if(episode.datetime_released != null) {
                 release_label = new Gtk.Label(episode.datetime_released.format(" %x"));
             } else {
-                release_label = new Gtk.Label(episode.date_released);
+                //We must fill the label with an empty string otherwise it prints "(null)"
+                release_label = new Gtk.Label("");
             }
             release_label.xalign = 0;
             release_label.justify = Gtk.Justification.LEFT;