[Bug 649040] Add volume control to qt-gstreamer example player

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=649040
  GStreamer | qt-gstreamer | 0.10.33

George Kiagiadakis <kiagiadakis.george> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #186945|none                        |reviewed
             status|                            |

--- Comment #1 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2011-04-30 18:30:41 UTC ---
Review of attachment 186945:
 --> (https://bugzilla.gnome.org/review?bug=649040&attachment=186945)

::: examples/player/mediaapp.cpp
@@ -195,1 +198,5 @@

+    m_volumeSlider = new QSlider(Qt::Horizontal);
+    m_volumeSlider->setTickPosition(QSlider::TicksLeft);
+    m_volumeSlider->setTickInterval(2);
+    m_volumeSlider->setMaximum(10);

Is just 10 levels of volume enough? I would expect something like 100...

@@ -196,0 +199,7 @@
+    m_volumeSlider = new QSlider(Qt::Horizontal);
+    m_volumeSlider->setTickPosition(QSlider::TicksLeft);
+    m_volumeSlider->setTickInterval(2);
... 4 more ...

I think it's fine to set the maximum size of a widget :)

::: examples/player/player.cpp
@@ +29,3 @@
 #include <QGst/Event>
+#include <QGst/StreamVolume>
+#include <QGst/enums.h>

Including enums.h is not required. It is included implicitly from global.h. In
general, all the *.h headers are considered private and should not be included
directly. One should only use the camel-case ones, just like with Qt.

@@ -98,2 +100,3 @@
 }

+int Player::getVolume()

Qt style: This method should be called just volume(), not getVolume().

@@ -100,0 +102,9 @@
+int Player::getVolume()
+{
+  if (m_pipeline) {
... 6 more ...

coding style: All this function should be re-indented to 4-space indentation,
like the rest of the file. Also, the if (svp) statement should have braces as
well (see http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Braces )

@@ -100,0 +102,19 @@
+int Player::getVolume()
+{
+  if (m_pipeline) {
... 16 more ...

coding style: as above, indent with 4 spaces and use braces in if(svp).

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.