[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-commits
Subject: Re: kdesupport/phonon
From: Marco Gulino <marco.gulino () gmail ! com>
Date: 2009-04-22 14:57:09
Message-ID: fc9d7c010904220757t59d0fcfbi1afc13615dff517d () mail ! gmail ! com
[Download RAW message or body]
Ok.. kinda got it (thought something is still not exactly clear).
Trying to do it the right way, i'll ask if any problem.
Thanks!
On Wed, Apr 22, 2009 at 3:55 PM, Matthias Kretz <kretz@kde.org> wrote:
> On Wednesday 22 April 2009 15:33:17 Marco Gulino wrote:
> > --- trunk/kdesupport/phonon/ds9/videowidget.cpp #957570:957571
> > @@ -384,9 +384,15 @@
> > renderer->notifyResize(m_widget->size(),
> > m_aspectRatio, m_scaleMode); }
> > }
> > +
> > + QImage VideoWidget::snapshot() const {
> > + // TODO implement me
> > + return QImage();
> > + }
>
> Don't do this. This will be forgotten and not implemented...
>
> Instead, if you create a new VideoWidget interface version this will be
> more
> obvious to backend developers.
>
> Same for the other backends.
>
> > --- trunk/kdesupport/phonon/phonon/videowidget.cpp #957570:957571
> > @@ -98,6 +98,9 @@
> > PHONON_INTERFACE_GETTER(qreal, saturation, d->saturation)
> > PHONON_INTERFACE_SETTER(setSaturation, saturation, qreal)
> >
> > +PHONON_INTERFACE_GETTER(QImage, snapshot, QImage() )
> > +
> > +
> > void VideoWidget::setFullScreen(bool newFullScreen)
> > {
> > pDebug() << Q_FUNC_INFO << newFullScreen;
> > --- trunk/kdesupport/phonon/phonon/videowidget.h #957570:957571
> > @@ -172,6 +172,7 @@
> > qreal contrast() const;
> > qreal hue() const;
> > qreal saturation() const;
> > + QImage snapshot() const;
>
> Please fix indentation. The Phonon sources use kdelibs (Qt) coding style.
>
> > //TODO: bar colors property
> > public Q_SLOTS:
> > --- trunk/kdesupport/phonon/phonon/videowidgetinterface.h #957570:957571
> > @@ -50,6 +50,7 @@
> > virtual qreal saturation() const = 0;
> > virtual void setSaturation(qreal) = 0;
> > virtual QWidget *widget() = 0;
> > + virtual QImage snapshot() const = 0;
>
> This breaks binary compatibility to the backends.
>
> You need to do the full magic here. For an example look at
> AudioOutputInterface. With that you also cannot use the
> PHONON_INTERFACE_GETTER macro anymore. Again look at AudioOutput how to do
> it.
>
> If you need help let me know, I will hack up a patch for you to start from.
>
> Regards,
> Matthias
>
> --
> ________________________________________________________
> Matthias Kretz (Germany) <><
> http://Vir.homelinux.org/
>
>
>
[Attachment #3 (text/html)]
Ok.. kinda got it (thought something is still not exactly clear).<br>Trying to do it the right \
way, i'll ask if any problem.<br>Thanks!<br><br><br><div class="gmail_quote">On Wed, Apr \
22, 2009 at 3:55 PM, Matthias Kretz <span dir="ltr"><<a \
href="mailto:kretz@kde.org">kretz@kde.org</a>></span> wrote:<br> <blockquote \
class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt \
0.8ex; padding-left: 1ex;">On Wednesday 22 April 2009 15:33:17 Marco Gulino wrote:<br> > --- \
trunk/kdesupport/phonon/ds9/videowidget.cpp #957570:957571<br> > @@ -384,9 +384,15 @@<br>
> renderer->notifyResize(m_widget->size(),<br>
> m_aspectRatio, m_scaleMode); }<br>
> }<br>
> +<br>
> + QImage VideoWidget::snapshot() const {<br>
> + // TODO implement me<br>
> + return QImage();<br>
> + }<br>
<br>
Don't do this. This will be forgotten and not implemented...<br>
<br>
Instead, if you create a new VideoWidget interface version this will be more<br>
obvious to backend developers.<br>
<br>
Same for the other backends.<br>
<br>
> --- trunk/kdesupport/phonon/phonon/videowidget.cpp #957570:957571<br>
> @@ -98,6 +98,9 @@<br>
> PHONON_INTERFACE_GETTER(qreal, saturation, d->saturation)<br>
> PHONON_INTERFACE_SETTER(setSaturation, saturation, qreal)<br>
><br>
> +PHONON_INTERFACE_GETTER(QImage, snapshot, QImage() )<br>
> +<br>
> +<br>
> void VideoWidget::setFullScreen(bool newFullScreen)<br>
> {<br>
> pDebug() << Q_FUNC_INFO << newFullScreen;<br>
> --- trunk/kdesupport/phonon/phonon/videowidget.h #957570:957571<br>
> @@ -172,6 +172,7 @@<br>
> qreal contrast() const;<br>
> qreal hue() const;<br>
> qreal saturation() const;<br>
> + QImage snapshot() const;<br>
<br>
Please fix indentation. The Phonon sources use kdelibs (Qt) coding style.<br>
<br>
> //TODO: bar colors property<br>
> public Q_SLOTS:<br>
> --- trunk/kdesupport/phonon/phonon/videowidgetinterface.h #957570:957571<br>
> @@ -50,6 +50,7 @@<br>
> virtual qreal saturation() const = 0;<br>
> virtual void setSaturation(qreal) = 0;<br>
> virtual QWidget *widget() = 0;<br>
> + virtual QImage snapshot() const = 0;<br>
<br>
This breaks binary compatibility to the backends.<br>
<br>
You need to do the full magic here. For an example look at<br>
AudioOutputInterface. With that you also cannot use the<br>
PHONON_INTERFACE_GETTER macro anymore. Again look at AudioOutput how to do it.<br>
<br>
If you need help let me know, I will hack up a patch for you to start from.<br>
<br>
Regards,<br>
Matthias<br>
<font color="#888888"><br>
--<br>
________________________________________________________<br>
Matthias Kretz (Germany) <><<br>
<a href="http://Vir.homelinux.org/" target="_blank">http://Vir.homelinux.org/</a><br>
<br>
<br>
</font></blockquote></div><br>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic