I think they should be included. If you don't like it move your podcast directory out of the music tree.

My own idea on a solution for this is the idea of having 2 libraries. One for my music collection and one for podcasts.

If I'm listening to my music library on shuffle, it's unlikely that I'll want to listen to a random episode of lugradio. Also, having both podcasts and music in the one library really clogs up the browser.

*** Bug 409575 has been marked as a duplicate of this bug. ***

*** Bug 447685 has been marked as a duplicate of this bug. ***

I'm of the opinion that this is NOTABUG.  The main library directory is supposed to be automatically scanned for audio files, if the user doesn't want their Podcasts in their main library, they should put the Podcast directory outside of the main library tree.  

It could be implemented to only allow a file to exist in one library at a time, but how do you determine the library precedence?  And what about users who WANT this sort of behaviour?

Created attachment 118193 [details] [review]
Ignore the podcasts under the music directory

I don't really like the code in rhythmdb-import-job.c, but it's required to avoid importing existing podcasts.

The best way to do this, after talking to Jonathan, would be to swap the location/mountpoint property use for podcasts, and podcasts would simply be ignored as they'd already be in the DB with a different entry type.

Created attachment 118194 [details] [review]
Better version

The hard part is, how do we swap the 2 properties in the DB, on startup.

And I'm not sure what that's supposed to do either...
↦       ↦       case RHYTHMDB_PROP_MOUNTPOINT:•
↦       ↦       ↦       /* fix old podcast posts */•
↦       ↦       ↦       if (g_str_has_prefix (ctx->buf->str, "http://"))•
↦       ↦       ↦       ↦       skip = TRUE;•
↦       ↦       ↦       break;•

Created attachment 118217 [details] [review]
Updated

A patch that compiles. The problem is that the rhythmdb just explodes when adding new podcasts. I think it really doesn't like us using the mountpoint as an identifier...

The location field always needs to be unique across all entries.  I guess we could use the origin location for episodes that haven't been downloaded, replacing it with the download location when we start the download.

(In reply to comment #9)
> And I'm not sure what that's supposed to do either...
> ↦       ↦       case RHYTHMDB_PROP_MOUNTPOINT:•
> ↦       ↦       ↦       /* fix old podcast posts */•
> ↦       ↦       ↦       if (g_str_has_prefix (ctx->buf->str,
> "http://"))•
> ↦       ↦       ↦       ↦       skip = TRUE;•
> ↦       ↦       ↦       break;•

Initially we stored the origin location in the mountpoint property, then replaced it with the downloaded location (sounds familiar..).  This code was added to clean up existing podcast entries.  This was almost three years ago now, so I think we can safely drop it now.

Created attachment 118334 [details] [review]
Done properly

The only thing missing is the upgrade code. We need to be able to swap location and mountpoint for podcast-posts when there's both in an entry, and remove normal "song" entries for those same podcasts.

I have no idea how to code that, but I added test code for it...

I guess you'd do that in rhythmdb_tree_parser_end_element under the RHYTHMDB_TREE_PARSER_STATE_ENTRY case.  Since podcast entries are always read after song entries, you could remove song entries just by doing a lookup in ctx->db->priv->entries (like the existing duplicate entry check) and deleting any song entry you find.

It might be worth considering combining the play count and last-played data from the song entry, too, but I don't think that's essential.

Created attachment 118398 [details] [review]
Upgrade the database as well

Upgrading now works using test-rhythmdb, need to test the whole solution now.

Created attachment 118416 [details] [review]
Mark the entry as not inserted when deleting it

This still doesn't work as expected

Created attachment 118427 [details] [review]
seems to work

The inserted flag is set in process_added_entries_cb, so it needs to be cleared in process_deleted_entries_cb.  This works in the very limited test setup I had.

Created attachment 118429 [details] [review]
And updated again

2008-09-11  Bastien Nocera  <hadess@hadess.net>

	* podcast/rb-feed-podcast-properties-dialog.c
	(rb_feed_podcast_properties_dialog_update_location):
	* podcast/rb-podcast-manager.c (get_download_location),
	(set_download_location), (get_remote_location),
	(rb_podcast_manager_download_entry),
	(rb_podcast_manager_entry_downloaded),
	(rb_podcast_manager_head_query_cb), (download_error),
	(rb_podcast_manager_next_file), (download_file_info_cb),
	(rb_podcast_manager_save_metadata), (download_progress),
	(podcast_download_thread), (end_job), (cancel_job),
	(rb_podcast_manager_db_entry_deleted_cb),
	(remove_if_not_downloaded):
	* podcast/rb-podcast-properties-dialog.c
	(rb_podcast_properties_dialog_update_location),
	(rb_podcast_properties_dialog_update_download_location):
	* rhythmdb/rhythmdb-tree.c (rhythmdb_tree_parser_start_element),
	(rhythmdb_tree_parser_end_element):
	* rhythmdb/rhythmdb.c (process_deleted_entries_cb):
	Forbid podcasts from showing up in the main library, by using
	the local download location to identify the file as soon as we start
	downloading it (Closes: #323440)

	* tests/podcast-upgrade.xml:
	* tests/test-rhythmdb.c (START_TEST), (rhythmdb_suite), (main):
	Add a test for the database upgrade code above

*** Bug 437701 has been marked as a duplicate of this bug. ***

*** Bug 463685 has been marked as a duplicate of this bug. ***