After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 427975 - sonyericcson w950i playlist support
sonyericcson w950i playlist support
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Removable Media
0.10.0
Other All
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on: 484768
Blocks:
 
 
Reported: 2007-04-09 19:09 UTC by Florian Meister
Modified: 2007-12-19 11:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
rb-mp3-player-parse-playlists-with-base.patch (4.25 KB, patch)
2007-11-07 12:20 UTC, Bastien Nocera
committed Details | Review
GDB log (459.51 KB, text/plain)
2007-11-08 21:57 UTC, Victor Osadci (Vic)
  Details

Description Florian Meister 2007-04-09 19:09:15 UTC
Please describe the problem:
can't use playlists in rhythmbox on my w950i.

Steps to reproduce:
1. 
2. 
3. 


Actual results:


Expected results:


Does this happen every time?


Other information:
the device uses normal m3u playlists. here is a snipplet of one generated on the mobile:

--snip--
#M3U

D:\music\01 - New Born.mp3
--snap--

the playlists are stored in /music/playlists, music is stored under /music. 

as you see, the m3u entries are stored with absolute "windows" paths, so this has to be implemented somehow.
Comment 1 Bastien Nocera 2007-11-07 12:18:01 UTC
The problem might be due to the root of the playlists and music being different:
<append key="portable_audio_player.audio_folders" type="strlist">Music/</append>
<append key="portable_audio_player.playlist_path" type="strlist">Music/playlists</append>

The playlist items relative paths would be resolved against Music/playlists, and not against the root of the device, or the music directory.

I committed a fix in Totem trunk:
2007-11-07  Bastien Nocera  <hadess@hadess.net>

        * src/plparse/totem-pl-parser-lines.c: (totem_pl_parser_add_m3u):
        Add support for parsing M3U files with drive letters, as used on
        some devices like the Sony Ericsson W950i (See #427975)

Rhythmbox now needs to use totem_pl_parser_parse_with_base().
Comment 2 Bastien Nocera 2007-11-07 12:20:12 UTC
Created attachment 98715 [details] [review]
rb-mp3-player-parse-playlists-with-base.patch

Untested (but compiling). This should (along with the fix in Totem) allow parsing of playlists with drive letters (resolved relative to the root of the device).
Comment 3 Victor Osadci (Vic) 2007-11-08 21:30:08 UTC
Some random comments:

* The playlist is shown with the .m3u extension in the rb side pane;

* The files in the playlist do play properly;

* Creating a new playlist in rb does not create a file on the device. The playlist is visible in rb until unmounting the device;

* Dragging files to the rb playlist does nothing, but a "+" mouse cursor is shown;

* Other text files that are not in the "music" or "playlist" dir. are shown as playlists;

* Other non music files that are not in the "music" dir are imported, which fails;

* Ejecting the device from rb makes rb crash - will investigate later;

Totem and Rhythmbox from trunk.
Comment 4 Victor Osadci (Vic) 2007-11-08 21:42:51 UTC
It seams there was an error in /usr/share/hal/fdi/information/10freedesktop/10-usb-music-players.fdi
the music dir was set to "Music" instead of "music". Any idea how to get this upstream ?

It would also be nice to have the device model shown in rb, not some generic "USB mass storage audio device"; any idea how to do this ?
Comment 5 Victor Osadci (Vic) 2007-11-08 21:57:04 UTC
Created attachment 98788 [details]
GDB log

to reproduce the crash:
1. Play a file from a playlist on the device;
2. Unmount/Eject the device while the file is playing.
Comment 6 Bastien Nocera 2007-11-08 23:33:50 UTC
(In reply to comment #3)
> Some random comments:
> 
> * The playlist is shown with the .m3u extension in the rb side pane;

Ok. We should strip the extension if there is one.

> * The files in the playlist do play properly;

Cool, that means the playlist is parsed properly.

> * Creating a new playlist in rb does not create a file on the device. The
> playlist is visible in rb until unmounting the device;

save_playlist() in rb-generic-player-playlist-source.c implements that. It will only create a new playlist if there's items being added (which means the playlist is marked as dirty).

Did you add files to the playlist? If not, then it's normal. We should really mark the source as dirty when it's created, not only when it has items.

> * Dragging files to the rb playlist does nothing, but a "+" mouse cursor is
> shown;

Could you file another bug about this? Does adding new songs to the top-level library for the device work?

> * Other text files that are not in the "music" or "playlist" dir. are shown as
> playlists;

Does that still happen when you changed the directory in HAL as mentioned in comment #5? We should skip every item that's not a playlist, it might need changes to totem's playlist parser to make this easier.

> * Other non music files that are not in the "music" dir are imported, which
> fails;

Same comment as above, did you check the case-sensitivity of the playlist directory?

> * Ejecting the device from rb makes rb crash - will investigate later;

Could you file this as another bug? I'm sure there's duplicates for it.

> Totem and Rhythmbox from trunk.

I guess it's Rhythmbox with the patch above applied, right?

(In reply to comment #4)
> It seams there was an error in
> /usr/share/hal/fdi/information/10freedesktop/10-usb-music-players.fdi
> the music dir was set to "Music" instead of "music". Any idea how to get this
> upstream ?

Not really "Music" is the correct directory name, but we should look for it better than that, and be case-unsensitive. VFAT sucks :)

Fixes would be in set_playlist_path(), and get_device_info() for the "audio_folders".

> It would also be nice to have the device model shown in rb, not some generic
> "USB mass storage audio device"; any idea how to do this ?

No idea. I'd need to try that out.
Comment 7 Victor Osadci (Vic) 2007-11-09 10:32:50 UTC
(In reply to comment #6)

> 
> > * Creating a new playlist in rb does not create a file on the device. The
> > playlist is visible in rb until unmounting the device;
> 
> save_playlist() in rb-generic-player-playlist-source.c implements that. It will
> only create a new playlist if there's items being added (which means the
> playlist is marked as dirty).
> 
> Did you add files to the playlist? If not, then it's normal. We should really
> mark the source as dirty when it's created, not only when it has items.
> 
> > * Dragging files to the rb playlist does nothing, but a "+" mouse cursor is
> > shown;
> 
> Could you file another bug about this? Does adding new songs to the top-level
> library for the device work?

Yes, it works when adding files to the top level.
Another thing I noticed - Existing playlists look like velid drop targets, but files are not added to them; new playlists are not valid drop targets.

> 
> > * Other text files that are not in the "music" or "playlist" dir. are shown as
> > playlists;
> 
> Does that still happen when you changed the directory in HAL as mentioned in
> comment #5? We should skip every item that's not a playlist, it might need
> changes to totem's playlist parser to make this easier.
> 
> > * Other non music files that are not in the "music" dir are imported, which
> > fails;
> 
> Same comment as above, did you check the case-sensitivity of the playlist
> directory?

Makes no difference.

> 
> > * Ejecting the device from rb makes rb crash - will investigate later;
> 
> Could you file this as another bug? I'm sure there's duplicates for it.
> 
> > Totem and Rhythmbox from trunk.
> 
> I guess it's Rhythmbox with the patch above applied, right?
> 

Yep, patch applied.


Comment 8 Jonathan Matthew 2007-11-11 10:33:32 UTC
I'm not sure from your descriptions above, but it sounds like you're trying to drag songs from the main library to a playlist on the device.  This hasn't been implemented yet.  You can only add songs that are already on the device to device playlists.
Comment 9 Bastien Nocera 2007-12-03 13:37:24 UTC
2007-12-03  Bastien Nocera  <hadess@hadess.net>

        * plugins/generic-player/rb-generic-player-playlist-source.c:
        (save_playlist), (handle_playlist_start_cb), (load_playlist),
        (rb_generic_player_playlist_source_new),
        (rb_generic_player_playlist_source_get_property),
        (rb_generic_player_playlist_source_set_property),
        (rb_generic_player_playlist_source_class_init):
        * plugins/generic-player/rb-generic-player-playlist-source.h:
        * plugins/generic-player/rb-generic-player-plugin.c:
        (rb_generic_player_plugin_new_playlist):
        * plugins/generic-player/rb-generic-player-source.c:
        (load_playlist_file), (default_load_playlists): Pass the device_root
        when parsing a playlist on a removable device, to allow drive letters
        in playlists (Helps: #427975)
Comment 10 Bastien Nocera 2007-12-19 11:21:32 UTC
(In reply to comment #6)
> (In reply to comment #3)
> > Some random comments:
> > 
> > * The playlist is shown with the .m3u extension in the rb side pane;
> 
> Ok. We should strip the extension if there is one.

Filed as bug #504423

<snip>
> (In reply to comment #4)
> > It seams there was an error in
> > /usr/share/hal/fdi/information/10freedesktop/10-usb-music-players.fdi
> > the music dir was set to "Music" instead of "music". Any idea how to get this
> > upstream ?
> 
> Not really "Music" is the correct directory name, but we should look for it
> better than that, and be case-unsensitive. VFAT sucks :)
> 
> Fixes would be in set_playlist_path(), and get_device_info() for the
> "audio_folders".

Filed as bug #504424

I'll close this, as playlist parsing works now. Please file the crasher on eject as a separate bug.