Question about the interface classes

[Matthias Kretz]

On Tuesday 26 June 2007, Håvard Wall wrote:
> On Friday 22 June 2007 18:35, Matthias Kretz wrote:
> > On Friday 22 June 2007, Håvard Wall wrote:
> > > > - less/no multiple inheritance for backend classes
> > >
> > > Why is that? Currently you need to subclass both the interface classes
> > > and QObject. You could make the current interface classes inherit
> > > QObject, add the virtual functions, make dummy implementations.
> > > Wouldn't that be easier for everybody. Anyway, you have much more
> > > experience with phonon than me, so I'm probably missing something or
> > > are just plain stupid -)
> >
> > Let me check whether the reasons still make sense (after that many API
> > changes it might just be that there's currently no problematic case)...
> >
> > It's still there with Effect and specialized Effect subclasses. Before I
> > was in Oslo this problem was a lot more visible with the classes
> > AbstractMediaProducer -> MediaObject -> MediaQueue. Anyway this is the
> > problem:
> >
> > let's assume for every frontend class we define a backend interface that
> > inherits QObject:
> > Effect -> EffectInterface
> > VolumeFaderEffect -> VolumeFaderEffectInterface
> >
> > a) EffectInterface inherits QObject, VolumeFaderEffectInterface inherits
> > QObject
> > => obviously doesn't work because a VolumeFaderEffect implementation
> > would have to inherit both EffectInterface and VolumeFaderEffectInterface
> >
> > b) VolumeFaderEffectInterface inherits EffectInterface instead of QObject
> > => the backend implementation can inherit VolumeFaderEffectInterface now,
> > but it cannot inherit its implementation of Effect anymore since that one
> > inherits from EffectInterface.
>
> Yes, ok, I agree making the interfaces derive from QObject is a bad idea.
>
> > Therefore, in order to be able to use inheritance in the frontend classes
> > and the backend classes the interfaces may not inherit QObject.
> >
> > This problem can be solved by using Q_INTERFACES, which is a really nice
> > solution, that I only learned a few months after I changed everything to
> > use invokeMethod. So from there I gradually changed many interfaces to
> > use Q_INTERFACES instead - but not all since I thought it doesn't hurt
> > much and is even more flexible than Q_INTERFACES.
> >
> > BTW, there are 39 functions defined that way with ~1173 characters used
> > to write out the function signatures. I'm not sure whether this really
> > has a big impact on the binary size of the lib or backend.
>
> I did a test implementation: Removing all use of the BACKEND_GET/CALL
> macros and replaced them with INTERFACE_CALLs. I added new virtual
> functions (with trivial base implementations) to the interface classes, and
> added new interface classes that would need them.
>   This reduced the library size with 32KB on x86 and 16KB on ARM.

Interesting. OTOH there will be another saving in the backend which doesn't 
have to make all functions slots.

> I agree this is not a huge reduction, and I'll be happy if we find other
> ways of cutting down the library overhead instead.

Do you have an idea how to profile this? Disassembling the stripped lib and 
see how big the functions are?

> From an embedded 
> perspective I still think it's worthwile at least considering if some of
> the functions could be moved into the interface classes. In addition to the
> reduced overhead of the actual function, such a size reduction is likely to
> improve cache behavior as well.

You mean invokeMethod -> virtual function with moving? I agree, of course, 
that invokeMethod is the most expensive thing to do...

> FYI, I've attached the patch I used to do the size measurements. This is
> not a final patch I suggest being commited, it would need some clean-ups
> first.

You forgot to "svn add" the new interface headers. While looking at it, do you 
think we should remove
bool supportsVideo()
bool supportsOSD()
bool supportsSubtitles()
? 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?

-- 
________________________________________________________
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/20070626/737fc8eb/attachment.pgp