Question about the interface classes

Matthias Kretz kretz at kde.org
Thu Jun 28 10:10:16 CEST 2007


On Tuesday 26 June 2007, Håvard Wall wrote:
> On Tuesday 26 June 2007 15:11, Matthias Kretz wrote:
> > You mean invokeMethod -> virtual function with moving? I agree, of
> > course, that invokeMethod is the most expensive thing to do...
>
> Yes, I guess my patch was the extreme. If you don't think it's the right
> thing to do, we'll leave it for now. If there's a subset of the
> invokeMethods that you think makes sense to make as virtual functions, that
> fine too.

I'd go for converting the *Path, VideoWidget interfaces + the missing 
functions for Backend and MediaObject (where remainingTime should have 
totalTime() - currentTime() as default implementation).

The effects stuff I'm undecided about.

> > While looking at it, do you think we should remove
> > bool supportsVideo()
> > bool supportsOSD()
> > bool supportsSubtitles()
>
> How about making these functions inline and relying on a single getter in
> the backend? Something like this maybe:
>
> class BackendCapabilities
> {
> public:
>     enum CapabilitiesFlag { // probably a bad name
>         Audio = 0x1,
>         Video = 0x2,
>         Subtitles = 0x4,
>         ...
>     };
>     Q_DECLARE_FLAGS(CapabilitiesFlags, CapabilityFlag)
>
>     inline bool supportsVideo() const { return capabilities() & Video; }
>
> private:
>     CapabilitiesFlags capabilities() const;   // probably bad naming
> };
> Q_DECLARE_OPERATORS_FOR_FLAGS(BackendCapabilities::CapabilitiesFlags)
>
> class BackendInterface
> {
> public:
>      virtual BackendCapabilities capabilities() const = 0;
> };
>
> Not sure if it's a bad idea. Only works for boolean capabilities, and might
> not be consistent with the rest of the API. Currently doesn't reduce much
> size, but it's easy to add support for new supportsFeatureX() in the future
> without breaking binary compatibility (and without increasing the library
> size -).

That's right. For BC issues the enum is only needed on the backend side. To 
reduce lib size a little it can be used on the frontend, too as you wrote. 
Perhaps we should add such a flag based capability function to the 
backend/frontend API, even if we don't use it for now.

> > ? At least OSD currently is not in the public API and should really not
> > be there. Whether subtitles are optional I don't know, is video support
> > optional for a backend?
>
> Since Phonon allows you to play an audio file only, my personal opinion is
> that it makes sense to make video support optional. (I even think audio
> support could be optional, you might have a system with a screen but no
> audio output.)

If audio or video can be optional that means applications have to test whether 
the given capability is there and use the API differently depending on what 
is supported and what isn't. Ideally the app can be written ignoring what the 
backend supports and Phonon still does the right thing.

If we expose supportsAudio/supportsVideo in the public API then we're telling 
the app developer to please check it and act accordingly.

And regarding not showing video files in a file selector or so, the 
supportedMimeTypes() function should be enough. Ah, regarding MIME types, is 
that useful for Qt users at all? E.g. for KFileDialog you can set a list of 
MIME types as filter instead of a list of file extensions 
(http://api.kde.org/4.0-api/kdelibs-apidocs/kio/html/classKFileDialog.html#d238e6bff14a53b40371194a8eecb21a).

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz at gmx.net, kretz at kde.org,
Matthias.Kretz at urz.uni-heidelberg.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/phonon-backends/attachments/20070628/4e59ed5b/attachment.pgp 


More information about the Phonon-backends mailing list