Review of attachment 182034 [details] [review]:

Hello,

Thanks a lot for the patch and sorry for delaying this review. I have made some comments on some parts of the API which need fixing. The rest seems fine and I'll happily merge it once those small issues are fixed.

Best regards,
George

::: src/QGst/tuner.h
@@ +20,3 @@
+
+#include "global.h"
+#include "../QGlib/Object"

I'd prefer this to be #include "../QGlib/object.h" for consistency with the rest of the files.

@@ +31,3 @@
+    QGST_WRAPPER(TunerChannel)
+public:
+    enum TunerChannelFlag {

This enum and its flags declaration should be defined outside of the class scope, in enums.h

@@ +32,3 @@
+public:
+    enum TunerChannelFlag {
+      Input = 1<<0,

also the names will need to be changed to TunerChannelInput, etc..

@@ +56,3 @@
+public:
+    QString label() const;
+    QGlib::Value framerate() const;

This is a bit weird being a GValue. I suppose that this is always a GST_TYPE_FRACTION GValue containing the framerate? In this case, it would make sense to export it as QGst::Fraction instead of QGlib::Value...

@@ +67,3 @@
+public:
+    QList<TunerChannelPtr> channels() const;
+    TunerChannelPtr getChannel() const;

I think the name should be currentChannel() here.

@@ +68,3 @@
+    QList<TunerChannelPtr> channels() const;
+    TunerChannelPtr getChannel() const;
+    void setChannel(const TunerChannelPtr &channel);

...and setCurrentChannel() respectively.

@@ +69,3 @@
+    TunerChannelPtr getChannel() const;
+    void setChannel(const TunerChannelPtr &channel);
+    TunerChannelPtr findChannelByName(const QString &name) const;

Use const char* for string arguments. Channel names are not translatable strings anyway and in most cases the programmer uses a string literal here (i.e. findChannelByName("composite")), in which case QString adds an extra overhead.

@@ +72,3 @@
+
+    QList<TunerNormPtr> norms() const;
+    TunerNormPtr getNorm() const;

Similar to currentChannel(), I think this should be currentNorm()

@@ +73,3 @@
+    QList<TunerNormPtr> norms() const;
+    TunerNormPtr getNorm() const;
+    void setNorm(const TunerNormPtr &norm);

...and setCurrentNorm()

@@ +74,3 @@
+    TunerNormPtr getNorm() const;
+    void setNorm(const TunerNormPtr &norm);
+    TunerNormPtr findNormByName(const QString &name) const;

again, const char*

@@ +77,3 @@
+    
+    unsigned long frequency(const TunerChannelPtr &channel) const;
+    void setFrequency(unsigned long freq, const TunerChannelPtr &channel);

I think the channel argument here should be first, like in the C API.

Created attachment 267181 [details] [review]
 Add support for GStreamer tuner interface V2 (patch)

The original patch from Lukas Schrangl with changes according to the comments from George.
And a small test from me.

One important note:
In gst_tuner_find_channel_by_name (GstTuner * tuner, gchar * channel)
and gst_tuner_find_norm_by_name (GstTuner * tuner, gchar * norm)
for no good reason "channel" and "norm" parameters are not const gchar *,
so we have to make a copy.

This does not seem particularly useful as the GstTuner interface was removed for GStreamer 1.0 and qtgstreamer hopefully is ported soon too.