[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: RE: KDE/kdelibs/phonon
From: "Thierry Bastian" <thierry.bastian () trolltech ! com>
Date: 2008-03-14 8:33:59
Message-ID: 000001c885ae$265fdec0$731f9c40$ () bastian () trolltech ! com
[Download RAW message or body]
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@gmail.com]
Sent: jeudi 13 mars 2008 16:53
To: kde-commits@kde.org
Cc: phonon-backends@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@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@kde.org
https://mail.kde.org/mailman/listinfo/phonon-backends
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic