Review Request: phonon phive core frontend api

[Matěj Laitl]

> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Player.h, line 39
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87102#file87102line39>
> >
> >     Should be addAudioOutput and addVideoOutput if you want to keep them separate.

Why?


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Player.h, line 51
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87102#file87102line51>
> >
> >     Metadata should go in the source!
> >     
> >     Would be nice in Amarok that we can simply map Track to Phonon::Source in the metadata handling.

+1.


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Player.h, line 52
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87102#file87102line52>
> >
> >     no. Total can be unknown and it's a one line calculation.

But Phonon may know the length better than we in Amarok. I would personally like to use some kind fo remainingTime signals to implement bounded playback in Amarok.


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Player.h, line 53
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87102#file87102line53>
> >
> >     You want to know the required resolution. And no, it's not easy to calculate when it might change!

I'd like to ditch ticking from Amarok code in favor of some kind of remainingTime mechanism.


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Queue.h, line 21
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87104#file87104line21>
> >
> >     Provide some convenience functions for this.
> >     iterator, index, append, prepend, etc.

Do we really need em? We just use enqueue() and clear() in Amarok.


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/abstract/AbstractVideoOutput.h, line 16
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87114#file87114line16>
> >
> >     Add same effect (object), what happens? Ordering?
> >     
> >     Could make this a QSet

> Could make this a QSet

No, effect order matters and QSet doesn't preserve order.


> On Sept. 26, 2012, 7:42 p.m., Bart Cerneels wrote:
> > core/Queue.h, line 26
> > <http://git.reviewboard.kde.org/r/106566/diff/1/?file=87104#file87104line26>
> >
> >     Couldn't I put the same source in multiple times? Which one will you delete?
> >     
> >     Again, convenience functions needed.
> >     
> >     Or will the Queue be set?

> Or will the Queue be [a] set?

Cannot be, set doesn't preserve order and we really need it for a .. queue.


- Matěj


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


On Sept. 25, 2012, 11:06 a.m., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106566/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2012, 11:06 a.m.)
> 
> 
> Review request for Amarok and Phonon.
> 
> 
> Description
> -------
> 
> phonon phive core frontend api
> 
> 
> Diffs
> -----
> 
>   core/AudioDataOutput.h PRE-CREATION 
>   core/AudioDataOutput.cpp PRE-CREATION 
>   core/AudioOutput.h PRE-CREATION 
>   core/AudioOutput.cpp PRE-CREATION 
>   core/BackendCapabilities.h PRE-CREATION 
>   core/BackendCapabilities.cpp PRE-CREATION 
>   core/OutputEffect.h PRE-CREATION 
>   core/OutputEffect.cpp PRE-CREATION 
>   core/Player.h PRE-CREATION 
>   core/Player.cpp PRE-CREATION 
>   core/Queue.h PRE-CREATION 
>   core/Queue.cpp PRE-CREATION 
>   core/Source.h PRE-CREATION 
>   core/Source.cpp PRE-CREATION 
>   core/VideoDataOutput.h PRE-CREATION 
>   core/VideoDataOutput.cpp PRE-CREATION 
>   core/abstract/AbstractAudioOutput.h PRE-CREATION 
>   core/abstract/AbstractAudioOutput.cpp PRE-CREATION 
>   core/abstract/AbstractMediaStream.h PRE-CREATION 
>   core/abstract/AbstractMediaStream.cpp PRE-CREATION 
>   core/abstract/AbstractVideoOutput.h PRE-CREATION 
>   core/abstract/AbstractVideoOutput.cpp PRE-CREATION 
>   core/core.pro PRE-CREATION 
>   core/effects/SubtitleEffect.h PRE-CREATION 
>   core/effects/SubtitleEffect.cpp PRE-CREATION 
>   core/five_global.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/106566/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

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