KDE/kdelibs/phonon

[Thierry Bastian]

Hello Ian,

I just wonder what review board you're talking about...

Anyway, here are the points I found about this patch.

Good ones:
- it removes a load of commented code in the most used classes (I hate
commented code)
- it adds those functionality as optional ones thanks to MediaController so
that it doesn't break the whole thing.

Bad points:
- the functions you uncommented are old and reference videoPath and
audioPath which don't even exist any more.
- I think submitting something without having the implementation behind
(even a proof-of-concept) is rarely a good idea.


For the videoPath/audioPath, I suggest the following change we use the
Phonon::Path class instead. I'll submit that patch to svn ASAP.

We're branching Qt 4.4 today so I think we should branch phonon as well.

Thierry

-----Original Message-----
From: Ian Monroe [mailto:ian.monroe at gmail.com] 
Sent: jeudi 13 mars 2008 16:53
To: kde-commits at kde.org
Cc: phonon-backends at kde.org
Subject: KDE/kdelibs/phonon

SVN commit 785244 by ianmonroe:

Adding subtitle and audio channel selection support to the Phonon API, as 
discussed on the review board.

Phonon-xine support for this API is coming shortly, with Dragon Player 
later today or tomorrow.
CCMAIL:phonon-backends at kde.org


 M  +16 -4     addoninterface.h  
 M  +49 -0     mediacontroller.cpp  
 M  +62 -0     mediacontroller.h  
 M  +0 -39     mediaobject.cpp  
 M  +2 -93     mediaobject.h  
 M  +0 -3      mediaobject_p.h  
 M  +0 -77     mediaobjectinterface.h  
 M  +6 -1      objectdescription.cpp  
 M  +12 -7     objectdescription.h  
 M  +0 -54     tests/fakebackend/mediaobject.cpp  
 M  +0 -11     tests/fakebackend/mediaobject.h  


_______________________________________________
Phonon-backends mailing list
Phonon-backends at kde.org
https://mail.kde.org/mailman/listinfo/phonon-backends