[Bug 726325] RFC: Add tunneling support.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=726325
  GStreamer | gst-omx | git

Julien Isorce <julien.isorce> changed:

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

--- Comment #16 from Julien Isorce <julien.isorce at gmail.com> 2014-03-18 12:00:11 UTC ---
Review of attachment 271868:
 --> (https://bugzilla.gnome.org/review?bug=726325&attachment=271868)

::: omx/gstomxvideodec.c
@@ +51,3 @@
 #include "gstomxvideo.h"
 #include "gstomxvideodec.h"
+#include "gstomxvideoenc.h"

I guess this is a residual of your initial try and then you forgot to remove it
? Otherwise I do not see why it's useful

@@ +1146,3 @@
   gst_video_codec_state_unref (state);

+  pool = gst_video_decoder_get_buffer_pool (GST_VIDEO_DECODER (self));

I think you never unref the pool
(http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-get-buffer-pool)

@@ +1154,3 @@
+    if (gst_omx_setup_tunnel (self->dec_out_port, op->port) != OMX_ErrorNone)
+      GST_WARNING_OBJECT (self, "Failed to setup tunnel");
+  }

will it fallback to non-tunneling mode if it fails to setup a tunnel ?

@@ +2230,3 @@
+  if (self->dec_out_port->tunneled) {
+    frame->output_buffer = gst_buffer_new ();
+    gst_video_decoder_finish_frame (GST_VIDEO_DECODER (self), frame);

Try to just use gst_video_decoder_release_frame
http://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-release-frame

Because gst_video_decoder_finish_frame actually push the data.

Note that on the gstomxencoder side you do need to have
gst_omx_video_enc_handle_frame being called. In your other patch you just unref
the frame. Better to no have the chain to be called at all in case of
GST_PAD_PROBE_TYPE_BUFFER + tunnel

@@ +2367,3 @@
     GST_ERROR_OBJECT (self, "Failed to drain component: %s (0x%08x)",
         gst_omx_error_to_string (err), err);
+    g_mutex_unlock (&self->drain_lock);

please put this in another patch

@@ +2381,3 @@
+        g_cond_wait_until (&self->drain_cond, &self->drain_lock, wait_until);
+
+    if (!result)

please put this in another patch

@@ +2395,3 @@
   GST_VIDEO_DECODER_STREAM_LOCK (self);

   self->started = FALSE;

Is it possible to avoid starting the GstTask when tunneling
http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1937 ?
I mean is gst_omx_video_dec_loop necessary
http://cgit.freedesktop.org/gstreamer/gst-omx/tree/omx/gstomxvideodec.c#n1161 ?

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