patch proposal

[Thierry Bastian]

Hi Matthias,

It seems indeed that some decisions that were made when you were in Oslo are
in contradiction with my e-mail... I also prefer to have objects everywhere
especially when they are so cheap like they are here. I dislike QList<int>
or whatever... especially.

QByteArray: well we also have objects where the system will give us the
properties. More specifically DirectShow gives us the property names for the
effects in UTF-16. So having QByteArray would just be painful.


ObjectDescriptionData: well I have tested that in my fake backend. The only
thing you do is something like that (the attached file). And all I have
changed compiles/links with my fake backend anyway.

"Why does subclassing reduce the size?" ... instead of templates... Hell I
don't know. I must admit I don't feel too proud of that specific change
anyway. fromIndex was removed and putting the template back is clearly an
option. Again for that Håvard will tell us. If the gain is not significant I
also vote for going back to templates.


Thanks again for your time and openness.

Regards,
Thierry

-----Original Message-----
From: Matthias Kretz [mailto:kretz at kde.org] 
Sent: mardi 3 juillet 2007 17:06
To: phonon-backends at kde.org
Subject: Re: patch proposal

On Tuesday 03 July 2007, Thierry Bastian wrote:
> -there were some places where integers were used instead of object 
> (like for the AudioOutputDevice

you mean the backend interface here, right?

> and the Effects). This now uses objects. This is to me the biggest 
> point in this patch: having a truly object-oriented approach 
> everywhere.

Didn't come around to comment on your previous mail, but yes, that's a
change we decided on at the API review when I was in Oslo. Before, the
parameterValue functions were functions of EffectParameter, so it was truly
object oriented.

I'm happy to change it to something more object oriented again.

> -property names were defined as QByteArray. This is changed to QString 
> because QByteArray will be painful. And anyway the QHash<QByteArray,
> QVariant> is changed into QVariantMap.

Ah, I thought QByteArray is nicer because most of the time you'll be using
hard-coded strings there anyway.

Current code:
objdesc.property("icon");
would then become
objdesc.property(QLatin1String("icon"));

> -for the object description, the backend is now responsible for 
> creating ObjectDescriptionData instead if returning both indexes and 
> properties separately. This needs changes in the backend as well so 
> don?t expect it to compile out-of-the-box.

Sounds good. But you have not tested to implement that in backend yet, have
you? Because I often discarded seemingly good ideas after I tried to
implement it in the backend. (The fake backend is a good playground for such
things.)

> Things to reduce size (all experimental):
>
> -make qDebug a noop in release mode. Now in release we do #define 
> pDebug if
> (false) qDebug (this saved 5% - ie 8KB - on my windows machine)

Good.

> - ObjectDescription is no more a template. I defined subclasses for 
> it. We used this template just for having string typing. The 
> subclasses have the same meaning. The static fromIndex was removed 
> thanks to using ObjectDescription everywhere instead of integers. 
> Replacing the templates by simple class/sub-classes made the library 
> size shrink on my machine by again 7%. I haven?t touch the 
> ObjectDescriptionModel, so remove it from your project file if you intend
to be able to compile ;).

I once tried subclassing but it didn't work for some reason. That reason
might be gone with the current design. Why does subclassing reduce the size?
With the template the only function that was put into libphonon was
fromIndex, now it'll have to put 13 class symbols in there. Would it be
possible to keep it as template and still remove fromIndex?

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/
MatthiasKretz at gmx.net, kretz at kde.org,
Matthias.Kretz at urz.uni-heidelberg.de
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: objectdescrpiptions.txt
Url: http://mail.kde.org/pipermail/phonon-backends/attachments/20070704/24d641e5/attachment.txt