-----------------------------------------------------------
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
> 
>

_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to