Merge lp:~jerico-dev/rhythmbox/rb-ipod-support-video-podcast into lp:~ubuntu-desktop/rhythmbox/lucid

[jerico]

Introduces video podcast support for iPods, e.g. Nano 5G. The patch has also been posted upstream, see https://bugzilla.gnome.org/show_bug.cgi?id=412841#c3. It would be great, though, to have this in Ubuntu already until it's integrated upstream.

[Robert Ancell (robert-ancell)]

Does this patch affect the Rhythmbox on-disk database? I ask because if upstream does not take this patch or modifies it will there be compatibility issues with the database on upgrade?

review: Needs Information

[jerico (jerico-dev)]

Hi Robert, this patch introduces the new field RHYTHMDB_PROP_HAS_VIDEO. I had a quick email exchange with upstream maintainer Jonathan Matthew about the patch. He thinks that yes, there might be upgrade problems. He says: "This feature involves some changes that we need to be very careful with, so we really need to do this upstream first." And I agree with him.

Jonathan promised to get back to me with a more detailed review of the patch at the time I submitted the merge request. But I guess he didn't have time yet. BTW: It seems I'm not the only one interested in this change coming out: https://bugzilla.gnome.org/show_bug.cgi?id=363822.

[Robert Ancell (robert-ancell)]

Ok, rejecting this patch due to the upgrade problems. I look forward to seeing this feature in 11.04!

Unmerged revisions

3. By Florian Grandel <fgrandel@laptop-fg> on 2010-07-05

* debian/patches/95_rb_ipod_support_video_podcast.patch:
  - added support for video podcasts

2. By Florian Grandel <fgrandel@laptop-fg> on 2010-07-04

- Added a RhythmDB entry property "has-video" that will be true when a file contains a video track
- The "has-video" property will be queried when adding podcasts to an iPod. The file's media type will be set to "video podcast" if true. This is required to support iPod video podcasts, e.g. on iPod Nano 5G.

=== modified file 'debian/changelog'
--- debian/changelog	2010-06-22 07:22:14 +0000
+++ debian/changelog	2010-07-05 01:50:37 +0000
@@ -1,3 +1,10 @@
+rhythmbox (0.12.8-0ubuntu7.ppa1) lucid; urgency=low
+
+  * debian/patches/95_rb_ipod_support_video_podcast.patch:
+    - added support for video podcasts 
+
+ -- Florian Grandel <[e-mail]>  Sun, 04 Jul 2010 21:49:25 -0300
+
 rhythmbox (0.12.8-0ubuntu7) lucid-proposed; urgency=low
 
   * debian/control:

=== added file 'debian/patches/95_rb_ipod_support_video_podcast.patch'
--- debian/patches/95_rb_ipod_support_video_podcast.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/95_rb_ipod_support_video_podcast.patch	2010-07-05 01:50:37 +0000
@@ -0,0 +1,140 @@
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/plugins/ipod/rb-ipod-source.c rhythmbox-0.12.8.new/plugins/ipod/rb-ipod-source.c
+--- rhythmbox-0.12.8/plugins/ipod/rb-ipod-source.c	2010-07-04 21:13:24.737138097 -0300
++++ rhythmbox-0.12.8.new/plugins/ipod/rb-ipod-source.c	2010-07-04 21:13:25.387133090 -0300
+@@ -536,6 +536,7 @@
+ /* FIXME:  these should go away once we compile against new-enough libgpod */
+ #define MEDIATYPE_AUDIO         0x0001
+ #define MEDIATYPE_PODCAST       0x0004
++#define MEDIATYPE_VIDEO_PODCAST 0x0006
+ 
+ static Itdb_Track *
+ create_ipod_song_from_entry (RhythmDBEntry *entry, guint64 filesize, const char *mimetype)
+@@ -568,7 +569,11 @@
+ 	track->playcount = rhythmdb_entry_get_ulong (entry, RHYTHMDB_PROP_PLAY_COUNT);
+ 
+ 	if (rhythmdb_entry_get_pointer (entry, RHYTHMDB_PROP_TYPE) == RHYTHMDB_ENTRY_TYPE_PODCAST_POST) {
+-		track->mediatype = MEDIATYPE_PODCAST;
++		if (rhythmdb_entry_get_boolean (entry, RHYTHMDB_PROP_HAS_VIDEO)) {
++			track->mediatype = MEDIATYPE_VIDEO_PODCAST;
++		} else {
++			track->mediatype = MEDIATYPE_PODCAST;
++		}
+ 		track->time_released = rhythmdb_entry_get_ulong (entry, RHYTHMDB_PROP_POST_TIME);
+ 	} else {
+ 		track->mediatype = MEDIATYPE_AUDIO;
+@@ -1332,7 +1337,7 @@
+ 							    filename);
+ 		g_free (filename);
+ 
+-		if (song->mediatype == MEDIATYPE_PODCAST) {
++		if (song->mediatype == MEDIATYPE_PODCAST || song->mediatype == MEDIATYPE_VIDEO_PODCAST) {
+ 			add_to_podcasts (isource, song);
+ 		}
+ 		device = rb_ipod_db_get_device (priv->ipod_db);		
+@@ -1753,7 +1758,7 @@
+ 	g_hash_table_iter_init (&iter, priv->entry_map);
+ 	while (g_hash_table_iter_next (&iter, &key, &value)) {
+ 		Itdb_Track *track = value;
+-		if (track->mediatype == MEDIATYPE_PODCAST) {
++		if (track->mediatype == MEDIATYPE_PODCAST || track->mediatype == MEDIATYPE_VIDEO_PODCAST) {
+ 			num_podcasts++;
+ 		}
+ 	}
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/podcast/rb-podcast-manager.c rhythmbox-0.12.8.new/podcast/rb-podcast-manager.c
+--- rhythmbox-0.12.8/podcast/rb-podcast-manager.c	2010-07-04 21:13:24.067132790 -0300
++++ rhythmbox-0.12.8.new/podcast/rb-podcast-manager.c	2010-07-04 21:14:46.757761062 -0300
+@@ -1353,6 +1353,16 @@
+ 		g_value_unset (&val);
+ 	}
+ 
++	if (rb_metadata_has_video (md)) {
++		rb_debug("Found video!");
++	} else {
++		rb_debug("No video found!");
++	}
++	g_value_init (&val, G_TYPE_BOOLEAN);
++	g_value_set_boolean (&val, rb_metadata_has_video (md));
++	rhythmdb_entry_set (pd->priv->db, entry, RHYTHMDB_PROP_HAS_VIDEO, &val);
++	g_value_unset (&val);
++
+ 	rhythmdb_commit (pd->priv->db);
+ 
+ 	g_object_unref (md);
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/rhythmdb/rhythmdb.c rhythmbox-0.12.8.new/rhythmdb/rhythmdb.c
+--- rhythmbox-0.12.8/rhythmdb/rhythmdb.c	2010-07-04 21:13:22.049008675 -0300
++++ rhythmbox-0.12.8.new/rhythmdb/rhythmdb.c	2010-07-04 21:13:25.387133090 -0300
+@@ -1972,6 +1972,12 @@
+ 		g_value_unset (&val);
+ 	}
+ 
++	/* video flag */
++	g_value_init (&val, G_TYPE_BOOLEAN);
++	g_value_set_boolean (&val, rb_metadata_has_video (metadata));
++	rhythmdb_entry_set_internal (db, entry, TRUE, RHYTHMDB_PROP_HAS_VIDEO, &val);
++	g_value_unset (&val);
++
+ 	/* musicbrainz trackid */
+ 	set_metadata_string_with_default (db, metadata, entry,
+ 					  RB_METADATA_FIELD_MUSICBRAINZ_TRACKID,
+@@ -3546,6 +3552,9 @@
+ 				g_date_clear (&entry->date, 1);
+ 			break;
+ 		}
++		case RHYTHMDB_PROP_HAS_VIDEO:
++			entry->has_video = g_value_get_boolean (value);
++			break;
+ 		case RHYTHMDB_PROP_TRACK_GAIN:
+ 			g_warning ("RHYTHMDB_PROP_TRACK_GAIN no longer supported");
+ 			break;
+@@ -4404,6 +4413,7 @@
+ 			ENUM_ENTRY (RHYTHMDB_PROP_LAST_PLAYED, "Last Played (gulong) [last-played]"),
+ 			ENUM_ENTRY (RHYTHMDB_PROP_BITRATE, "Bitrate (gulong) [bitrate]"),
+ 			ENUM_ENTRY (RHYTHMDB_PROP_DATE, "Date of release (gulong) [date]"),
++			ENUM_ENTRY (RHYTHMDB_PROP_HAS_VIDEO, "Whether the file contains a video track (gboolean) [has-video]"),
+ 			ENUM_ENTRY (RHYTHMDB_PROP_TRACK_GAIN, "Replaygain track gain (gdouble) [replaygain-track-gain]"),
+ 			ENUM_ENTRY (RHYTHMDB_PROP_TRACK_PEAK, "Replaygain track peak (gdouble) [replaygain-track-peak]"),
+ 			ENUM_ENTRY (RHYTHMDB_PROP_ALBUM_GAIN, "Replaygain album pain (gdouble) [replaygain-album-gain]"),
+@@ -5366,6 +5376,8 @@
+ 	switch (propid) {
+ 	case RHYTHMDB_PROP_HIDDEN:
+ 		return ((entry->flags & RHYTHMDB_ENTRY_HIDDEN) != 0);
++	case RHYTHMDB_PROP_HAS_VIDEO:
++		return (entry->has_video);
+ 	default:
+ 		g_assert_not_reached ();
+ 		return FALSE;
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/rhythmdb/rhythmdb.h rhythmbox-0.12.8.new/rhythmdb/rhythmdb.h
+--- rhythmbox-0.12.8/rhythmdb/rhythmdb.h	2010-07-04 21:13:22.049008675 -0300
++++ rhythmbox-0.12.8.new/rhythmdb/rhythmdb.h	2010-07-04 21:13:25.387133090 -0300
+@@ -170,6 +170,7 @@
+ 	RHYTHMDB_PROP_LAST_PLAYED,
+ 	RHYTHMDB_PROP_BITRATE,
+ 	RHYTHMDB_PROP_DATE,
++	RHYTHMDB_PROP_HAS_VIDEO,
+ 	RHYTHMDB_PROP_TRACK_GAIN,			/* obsolete */
+ 	RHYTHMDB_PROP_TRACK_PEAK,			/* obsolete */
+ 	RHYTHMDB_PROP_ALBUM_GAIN,			/* obsolete */
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/rhythmdb/rhythmdb-private.h rhythmbox-0.12.8.new/rhythmdb/rhythmdb-private.h
+--- rhythmbox-0.12.8/rhythmdb/rhythmdb-private.h	2010-07-04 21:13:22.049008675 -0300
++++ rhythmbox-0.12.8.new/rhythmdb/rhythmdb-private.h	2010-07-04 21:13:25.387133090 -0300
+@@ -88,6 +88,7 @@
+ 	gulong duration;
+ 	gulong bitrate;
+ 	GDate date;
++	gboolean has_video;
+ 
+ 	/* filesystem */
+ 	RBRefString *location;
+diff -Nur -x '*.orig' -x '*~' rhythmbox-0.12.8/rhythmdb/rhythmdb-tree.c rhythmbox-0.12.8.new/rhythmdb/rhythmdb-tree.c
+--- rhythmbox-0.12.8/rhythmdb/rhythmdb-tree.c	2010-07-04 21:13:24.747134577 -0300
++++ rhythmbox-0.12.8.new/rhythmdb/rhythmdb-tree.c	2010-07-04 21:13:25.387133090 -0300
+@@ -1006,6 +1006,9 @@
+ 			else
+ 				save_entry_ulong (ctx, elt_name, 0, TRUE);
+ 			break;
++		case RHYTHMDB_PROP_HAS_VIDEO:
++			save_entry_boolean(ctx, elt_name, entry->has_video);
++			break;
+ 		case RHYTHMDB_PROP_DURATION:
+ 			save_entry_ulong (ctx, elt_name, entry->duration, FALSE);
+ 			break;