[Bug 703518] Qt5/QtQuick 2.0 video element

[GStreamer (bugzilla.gnome.org)]

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

George Kiagiadakis <kiagiadakis.george> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #248290|none                        |needs-work
             status|                            |

--- Comment #16 from George Kiagiadakis <kiagiadakis.george at gmail.com> 2013-10-11 14:14:20 UTC ---
Review of attachment 248290:
 --> (https://bugzilla.gnome.org/review?bug=703518&attachment=248290)

I don't like this patch in general.

1) As I have said, I would prefer VideoNode* inside qtvideosink.
2) I'd rather not expose those Abstract* classes. AbstractVideoSurface makes
sense maybe, but since we can't make it a superclass of GraphicsVideoSurface
(to preserve ABI), it's useless.
3) There is a lot of commented out code and code with "still needed?" comments
that should not be there.
4) What is the purpose of VideoNode_Generic?

::: qt-gstreamer-orig/src/QGst/Ui/quickviewvideosurface.h
@@ +62,3 @@
+     * Helper - returns true if the given orientation has the same aspect as
the default (e.g. 180*n)
+     */
+    inline bool isDefaultAspect(int o) {

These inline methods could be in the .cpp

-- 
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.