Fwd: Review Request: State machine architecture for PMC

Thu Apr 8 08:52:53 CEST 2010

No SVN access yet. This is the first time I work with you guys. Thanks for
commiting it. I'll read trhough your other mail now

On Thu, Apr 8, 2010 at 8:36 AM, Alessandro Diaferia
<alediaferia at gmail.com>wrote:

>
>
> > On 2010-04-07 22:25:15, Alessandro Diaferia wrote:
> > > Ok, things are getting a little huge. Let's see together how we can
> come out of this.
> > >
> > > I like the idea of the state machine and how it could help us correctly
> managing the workflow of the mediacenter. Unfortunately i feel something in
> the current implementation lacks of robustness (does this word exist?).
> > >
> > > As Aaron pointed out, we need to understend whether we are aproaching
> this the right way.
> > >
> > > The mistake with the MediaLayout in this patch well shows that we
> should probably re-design something. The need for states is due to the fact
> that we want PMC to know the current media in order to differently behave
> and interact with the user. This way we could give the best set of actions
> for each kind of media chosen by the user.
> > > The first clear issue was the lack for a way of changing the UI
> accordingly to the chosen media.
> > >
> > > I'd go with questions here in order to better summarize what we need in
> the PMC:
> > >
> > > Q1. When do we need the UI to react to media type changes?
> > > R1. The UI should show different possible actions to the user both
> while browsing to a personal collection of e.g. videos and when actually
> reproducing a video.
> > >
> > > Q2. How the PMC knows which kind of media are we browsing
> through/reproducing ?
> > > R2. The PMC::Browser API should be updated in order to expose this kind
> of information. In addition to this the PMC::Player should be used in order
> to retrieve this kind of information while reproducing media.
> > >
> > > Q3. Are media types mutually exclusive?
> > > R3. This (unfortunately?) cannot always be true. The user might want to
> start his favourite playlist and then run a slideshow of his summer photos.
> Probably the PMC::Player should inform whether music is currently playing in
> background. I'd like to know from you whether you think music is the only
> media type that can be played  in background while reproducing something
> else in foreground. This is an important topic IMHO.
> > >
> > > Q4. Which UI controls does the PMC need?
> > > R4. The PMC needs of course playback controls for the current media. In
> addition to this the browser should allow the user to navigate through his
> collection of medias and eventually get back to the welcome page. The
> welcome page should IMO be inside the browser.
> > >
> > >
> > > Answering the above questions i feel that what we already have is
> enough in order to make the UI adapting to what is happening inside PMC. The
> MediaController can be updated in order to support background and foreground
> media playback control. When MediaBrowser plugins will be ready the welcome
> screen will be there for free showing the user the available kind of medias.
> The MediaPlayer should be updated in order to both play music and slideshows
> that shouldn't be too hard to do. I really like the MediaToolbox by
> Cristophe and i think that its browsing part could directly go inside the
> MediaBrowser. Of course i might be wrong about all this stuff and so,
> please, tell me your opinions.
> > >
> > > I hope this is constructive enough to reach a valid design together.
> > >
> > > Regards.
> >
> > Aaron Seigo wrote:
> >     "As Aaron pointed out, we need to understend whether we are
> aproaching this the right way."
> >
> >     actually, what i was trying to point out is that this patch needs to
> go into svn or you may as well abandon it. this is a patch against something
> in playground, not in a shipping module, it's getting bigger and simply
> offering ideas of how to improve it at this point, esp on the details, is
> far far slower than doing it together directly in svn.
> >
> >     connectMediaLayout, for instance, is probably fixable quite quickly
> by you or someone else familiar with that code if it was in svn.
> alternately, we can sit here and stare at the code forever.
> >
> >     so if you think this is the right direction, the it needs to get
> triaged into svn and we can fix the rest of the issues there.
> >
>
> Ok, i wasn't probably clear enough. This patch should go in. I'd just like
> to see more control given to each PMC::Applet while using the states for UI
> transitions.
>
> So, Christophe, do you have SVN access? If not i'll commit for you this
> patch.
>
>
> - Alessandro
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3396/#review4918
> -----------------------------------------------------------
>
>
> On 2010-04-07 20:59:11, Christophe Olinger wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > http://reviewboard.kde.org/r/3396/
> > -----------------------------------------------------------
> >
> > (Updated 2010-04-07 20:59:11)
> >
> >
> > Review request for Plasma.
> >
> >
> > Summary
> > -------
> >
> > This is the first try to get state machines working. I will of course not
> forget the previous comments. It builds and runs and state switching is
> possible. No playbackbuttons or other useful stuff yet, but is easy to get
> back.
> >
> > What I did:
> > Created a mediacenterstate class which contains info about shared (so
> called Main) subcomponents. subcomponents are little widgets like buttons
> and sliders which will appear in the control bar. I sublcassed QState for
> this.
> > Then I created a picturestate and videostate class, subclasses of the
> follwoing which are loaded on state switch. The containment does the state
> switch, the state classes do all the rest: connections, conficurations and
> hading out subcomponents to the controlbar.
> > The controlbar now has a class to add widgets to itself.
> >
> >
> > Diffs
> > -----
> >
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/abstractmediaitemview.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediabrowser/mediabrowser.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediacontroller/controller.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/applets/mediaplayer/mediaplayer.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/mediacontainment.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/containments/mediacontainment/medialayout.cpp
> 1112197
> >   trunk/playground/base/plasma/MediaCenterComponents/libs/CMakeLists.txt
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/CMakeLists.txt
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/browser.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenter.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.h
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/mediacenterstate.cpp
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.h
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/musicstate.cpp
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.h
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/picturestate.cpp
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/playbackcontrol.cpp
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/player.h
> 1112197
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.h
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/libs/mediacenter/videostate.cpp
> PRE-CREATION
> >
> trunk/playground/base/plasma/MediaCenterComponents/shells/plasmediacenter/mainwindow.cpp
> 1112197
> >
> > Diff: http://reviewboard.kde.org/r/3396/diff
> >
> >
> > Testing
> > -------
> >
> > State switchting works.
> >
> >
> > Thanks,
> >
> > Christophe
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100408/e3bab1ed/attachment.htm 