Review Request: State machine architecture for PMC

Aaron Seigo aseigo at kde.org
Thu Apr 8 06:43:40 CEST 2010 


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

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


- Aaron


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

More information about the Plasma-devel mailing list