Re: [Rhythmbox-devel] [PATCH] Podcast support

On Thu, 2005-09-01 at 11:35 -0300, Renato Araujo wrote:
> Hi all,
> I finally had time to finish the initial podcast support for
> rhythmbox. Attached there are the diff against the CVS HEAD and a
> tarball with the new files. To test it just apply the patch and
> decompress the tarball inside your rhythmbox devel src dir.
> Please let me know what you think about it. Any doubt and suggestions
> are welcome. I will be glad to fix the bugs and add new features as
> needed.

Ok cool...some more comments:

> +	a_val = rhythmdb_entry_get_string (a, data->propid);
> +	strptime(a_val, "%d/%m/%Y %H:%M", &a_tm);
> +	a_tm.tm_sec = 0;
> +	a_time = mktime(&a_tm);

This is going to be slow.  You should store the date as a regular gulong
too, and hook into the rhythmdb property mirroring to use the string as
a cache of the gulong property.  Look at how mirroring from

Now on to the new files:

 Need to handle errors from strftime.
 Couldn't you use gnome_vfs_read_entire_file?  Also why not use SAX like
 rhythmdb-tree.c does instead of reading the whole thing in at once?

 Calling the rhythmdb functions here from the thread might work, but
 it's not recommended anymore.  Doing the GDK_THREADS_ENTER and
 g_signal_emit is *definitely* not recommended.  We went down the route
 before of having lots of threads entering the UI, it was just too
 crazy.  I suggest creating a thread which just grabs the data from the
 network or whatever, and hands it off to the main UI.  Actually it
 looks like you could just use the gnome_vfs async operations here.
 Alternatively you might look at lib/rb-thread.[ch], which I wrote a
 while ago as a generic way to run a function in another thread, and
 ship its result back to the mainloop.  It hasn't been heavily tested
 yet though =)

 Should use gnome_vfs_make_uri_from_input.

Also on an overall stylistic note, the patch and new code has a lot of
missing spaces before parens; e.g. 


should be:

gtk_widget_hide (GTK_WIDGET (gtkdialog));

Yeah, it's annoying, but that's been the coding style since before I was
involved, and it's important to keep it consistent.

Now...onto the user interface.  You'll have to bear with me because up
until 30 seconds ago when I tried your patch I'd never actually never
listened to a podcast.  It seems like from browsing the web, the
"typical" way is that a web site gives a link, and tells you to copy it
to your music software.  Is that right?

Just giving the user a URL entry box seems kind of unfriendly.  Is there
anything we can do to make this better?  

It looks like some web sites support dragging an image from the site
into iTunes.  I wonder how that works exactly?  Maybe metadata embedded
in the image data?  Could we parse that?

It'd be nice at least to at least structure the import podcast dialog

Podcast information
  Link (RSS): [ entrybox here ]
  [X] Download updates for this podcast

Attachment: signature.asc
Description: This is a digitally signed message part

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]