KDE/kdelibs/phonon

[Thierry Bastian]

Ok, my bad for not following the review process.

For the code example, it looks like what we agreed on. I'm not sure about
all the details and we probably need to try implementing to get all the
thing up and running.

Why should things be moved from MediaController to MediaObject. I guess it
depends on what manages the ports. Would that be an optional feature and be
managed through MediaController as well ?
I definitely need to think about all that.

Anyway thanks for your time and your explanations.

Thierry



-----Original Message-----
From: Matthias Kretz [mailto:kretz at kde.org] 
Sent: vendredi 14 mars 2008 12:04
To: phonon-backends at kde.org
Subject: Re: KDE/kdelibs/phonon

On Friday 14 March 2008, Thierry Bastian wrote:
> Hello Ian,
>
> I just wonder what review board you're talking about...

He means the two mails that he sent to phonon-backends linking to the review
board website where you can see and comment on the patch (after
registering). 
It was a bit of an experiment to use the website for this.

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

You're right, of course. I didn't take enough time to think about it and
only remembered my old thoughts (pre-graph-discussions) on this. Ian only
added the path arguments because I told him to keep them. My fault.

> - I think submitting something without having the implementation 
> behind (even a proof-of-concept) is rarely a good idea.

Ian has an implementation for phonon-xine + API use in dragonplayer. Except
for the path arguments...

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

On Friday 14 March 2008, Thierry Bastian wrote:
> It goes against what we already agreed on: it is not on the path that 
> we set which audio/video stream should be used. We do that on the 
> outputs of the mediaobject (the ports). By default , the MediaObject 
> has one output for each type of media (Audio and Video for now, subtitle
in the future).

Wow. Seems my brain is a black hole. I'm glad I still have
phonon/DESIGN/Phonon-Graph.cpp to get those thoughts back. OK, so we want to
have the following:

MediaObject *media = new MediaObject;
media->setCurrentSource(Phonon::Dvd);
VideoWidget *vwidget = new VideoWidget(parent); AudioOutput *aoutput = new
AudioOutput; Path path = Phonon::createPath(media, vwidget); //
path.mediaStreamTypes() == Phonon::Video | Phonon::Subtitles
Phonon::createPath(media, aoutput); QList<SubtitleStreamDescription> l =
media->availableSubtitleStreams();
// either:
media->setCurrentSubtitleStream(l.first());
// or:
Port port1 = media->openPort(Phonon::Subtitles);
media->setCurrentSubtitleStream(l.at(0), port1);
Port port2 = media->openPort(Phonon::Subtitles);
media->setCurrentSubtitleStream(l.at(1), port2);

// in the latter case you probably want an explicit path for subtitles:
path.reconnect(media, Port(Phonon::Video), vwidget);
Phonon::createPath(media, port1, vwidget); VideoWidget *vwidget2 = new
VideoWidget(parent); Phonon::createPath(media, Port(Phonon::Video),
vwidget2); Phonon::createPath(media, port2, vwidget2);

Is that what we wanted?

Then some of the work Ian did needs to move back from MediaController to
MediaObject and all the path references in those functions need to be
removed.

What about the video angle functions in MediaController? Should we deprecate
them and add them as *VideoStream* functions in MediaObject?

--
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz at gmx.net, kretz at kde.org,
Matthias.Kretz at urz.uni-heidelberg.de