Review Request: Make sure Amarok can enqueue local files of all supported Media types in the playlist.

[Ralf Engels]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103694/#review9845
-----------------------------------------------------------


Hi,
good idea to let the backends check if they can play the files.

However I don't like the processEvents an the while loop.

1. the local processEvents is unexpected as it's very seldom used outside modal dialogs.
It will lead to potentially events being processed while processing an other event.

2. If no event is pending the while loop is a busy wait.
Drain on processor.

I don't have the details but I perfere an other solution.

- Ralf Engels


On Jan. 14, 2012, 2:20 p.m., Shlomi Fish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103694/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2012, 2:20 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> This is a patch to https://bugs.kde.org/show_bug.cgi?id=267319 , where Amarok refuses to enqueue many media file types that are supported by the Phonon backend.
> 
> It fixes the problem on Amarok by instantiating a
> Phonon::MediaObject object and trying to see if it can load the local file. It
> comes with the following reservations:
> 
> 1. With Phonon-VLC all files (including non-media ones) are accepted. This
> appears to affect dragonplayer as well, and seems to be a Phonon::MediaObject's
> ->setCurrentSource() related bug. ( I'll report it later. )
> 
> 2. I didn't try to enqueue playlist files yet.
> 
> 3. On my x86-64 Mageia Linux cauldron laptop, the phonon-gstreamer backend
> often crashes Amarok. However, the same files also crash gst123.
> 
> 4. I added some traces there to test something back when I was using phonon-vlc
> - they are no longer needed.
> 
> Anyway, please look into it and see if you like the direction I'm taking.
> 
> Regards,
> 
> -- Shlomi Fish
> 
> 
> Diffs
> -----
> 
>   src/EngineController.cpp 81f39b7 
> 
> Diff: http://git.reviewboard.kde.org/r/103694/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Shlomi Fish
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120115/e3c66d0c/attachment.html>