This is an automatically generated e-mail. To reply, visit: http://git.revie= wboard.kde.org/r/106566/

On September 26th, 2012, 7:42 p.m., Bart Ce= rneels wrote:
= = =
core/Player.h (Diff revision 1)
public:
39
    bool addOutput(Abstract=
AudioOutput *audioOutput<=
/span>);
Should be=
 addAudioOutput and addVideoOutput if you want to keep them separate.

core/Player.h (Diff revision 1)
	public:
		39	bool addOutput(Abstract= AudioOutput *audioOutput<= /span>);

Why?

On September 26th, 2012, 7:42 p.m., Bart Ce= rneels wrote:
= = =
core/Player.h (Diff revision 1)
public:
51
#warning metadata missing
Metadata =
should go in the source!

Would be nice in Amarok that we can simply map Track to Phonon::Source in t=
he metadata handling.

core/Player.h (Diff revision 1)
	public:
		51	#warning metadata missing

+1.

On September 26th, 2012, 7:42 p.m., Bart Ce= rneels wrote:
= = =
core/Player.h (Diff revision 1)
public:
52
#warning remainingTime inte=
rface?
no. Total=
 can be unknown and it's a one line calculation.

core/Player.h (Diff revision 1)
	public:
		52	#warning remainingTime inte= rface?

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

On September 26th, 2012, 7:42 p.m., Bart Ce= rneels wrote:
= = =
core/Player.h (Diff revision 1)
public:
53
#warning ticking missing
You want =
to know the required resolution. And no, it's not easy to calculate whe=
n it might change!

core/Player.h (Diff revision 1)
	public:
		53	#warning ticking missing

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

On September 26th, 2012, 7:42 p.m., Bart Ce= rneels wrote:
= = =
core/Queue.h (Diff revision 1)
class Queue {
21
        void enqueue(const<=
/span> Source &source, const=
 Source &before=3DSou=
rce());
Provide s=
ome convenience functions for this.
iterator, index, append, prepend, etc.

core/Queue.h (Diff revision 1)
	class Queue {
		21	void enqueue(const<= /span> Source &source, const= Source &before=3DSou= rce());

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




 On September 26th, 2012, 7:42 p.m., Bart Ce=
rneels wrote:
 
  =









 =

 =




 
 
  
   
    core/Queue.h
    

     (Diff revision 1)

    
   
  
 

 
  

   
   class Queue {

  
 


  
    
    
    26
            Source dequeue(const=
 Source &source =3D S=
ource());
  

 



  Couldn=
9;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 .. qu=
eue.




 On September 26th, 2012, 7:42 p.m., Bart Ce=
rneels wrote:
 
  =









 =

 =




 
 
  
   
    core/abstract/AbstractVideoOutput.h
    

     (Diff revision 1)

    
   
  
 

 
  

   
   private:

  
 


  
    
    
    16
        QList<OutputEffect *=
> m_effects;
  

 



  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.






- Mat=C4=9Bj




On September 25th, 2012, 11:06 a.m., Harald Sitter wrote:







 
  

Review request for Amarok and Phonon.
By Harald Sitter.


Updated Sept. 25, 2012, 11:06 a.m.






Descripti=
on 
 
 
  
   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)<=
/span>

 core/BackendCapabilities.cpp (PRE-CREATION=
)

 core/OutputEffect.h (PRE-CREATION)<=
/li>

 
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-C=
REATION)

 core/abstract/AbstractAudioOutput.cpp (PRE=
-CREATION)

 core/abstract/AbstractMediaStream.h (PRE-C=
REATION)

 core/abstract/AbstractMediaStream.cpp (PRE=
-CREATION)

 core/abstract/AbstractVideoOutput.h (PRE-C=
REATION)

 core/abstract/AbstractVideoOutput.cpp (PRE=
-CREATION)

 core/core.pro (PRE-CREATION)

 core/effects/SubtitleEffect.h (PRE-CREATIO=
N)

 core/effects/SubtitleEffect.cpp (PRE-CREAT=
ION)

 core/five_global.h (PRE-CREATION)



View Diff

core/Queue.h (Diff revision 1)
	class Queue {
		26	Source dequeue(const= Source &source =3D S= ource());

Descripti= on

Diffs=