[Bug 768110] new plugin: ahssrc (Android hardware sensor source)

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=768110

Olivier Crête <olivier.crete at ocrete.ca> changed:

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

--- Comment #14 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 331379:
 --> (https://bugzilla.gnome.org/review?bug=768110&attachment=331379)

::: sys/androidmedia/gstahssrc.c
@@ +613,3 @@
+  /* If the clock is NULL, the pipeline is not yet set to PLAYING. */
+  if (pipeline_clock == NULL)
+    goto done;

You should also provide the system clock as a backup by implementing the  (see
how rtpjitterbuffer does it). If you have ahssrc ! appsink, there will be no
element to provide one.

@@ +629,3 @@
+  gst_object_ref (pipeline_clock);
+  buffer_time = gst_clock_get_time (pipeline_clock);
+  gst_object_unref (pipeline_clock);

No need to ref & unref the clock, you're still holding the element's object
lock.

@@ +636,3 @@
+  else
+    buffer_duration = GST_CLOCK_TIME_NONE;
+  self->previous_time = buffer_time;

I would just skip the duration entirely and always set it to NONE. Those events
don't really have a duration.

--- Comment #15 from Olivier Crête <olivier.crete at ocrete.ca> ---
Review of attachment 331379:
 --> (https://bugzilla.gnome.org/review?bug=768110&attachment=331379)

::: sys/androidmedia/gstahssrc.c
@@ +613,3 @@
+  /* If the clock is NULL, the pipeline is not yet set to PLAYING. */
+  if (pipeline_clock == NULL)
+    goto done;

You should also provide the system clock as a backup by implementing the  (see
how rtpjitterbuffer does it). If you have ahssrc ! appsink, there will be no
element to provide one.

@@ +629,3 @@
+  gst_object_ref (pipeline_clock);
+  buffer_time = gst_clock_get_time (pipeline_clock);
+  gst_object_unref (pipeline_clock);

No need to ref & unref the clock, you're still holding the element's object
lock.

@@ +636,3 @@
+  else
+    buffer_duration = GST_CLOCK_TIME_NONE;
+  self->previous_time = buffer_time;

I would just skip the duration entirely and always set it to NONE. Those events
don't really have a duration.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.