[Bug 731769] add element implementing the ONVIF RTP extension

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=731769
  GStreamer | gst-plugins-bad | unspecified

--- Comment #2 from Guillaume Desmottes <gdesmott at gnome.org> 2014-06-24 12:53:48 UTC ---
(In reply to comment #1)
> Review of attachment 278581 [details]:
> 
> This generally looks good, but it introduces a one frame latency without ever
> declaring it. Which is quite annoying to do because it's in RTP land and we
> have no idea of the buffer rate.

That's why we implemented the 'set-e-bit' property, defaulting to FALSE. While
it's not set no latency is introduced.

> ::: gst/onvif/gstonvif.c
> @@ +31,3 @@
> +{
> +  if (!gst_element_register (plugin, "onviftimestamp_apply", GST_RANK_NONE,
> +          GST_TYPE_ONVIF_TIMESTAMP_APPLY))
> 
> I don't like the name too much, any reason not to name it "rtponviftimestamp"
> for example? That would say it's about RTP, and you can use "timestamp" as a
> verb"

We have some plan to have the reverse, client side, element which would be
called "onviftimestamp_extract". Suggestions for better names welcome.

> ::: gst/onvif/gstonviftimestamp-apply.c
> @@ +212,3 @@
> +    gst_event_copy_segment (event, &self->segment);
> +  } else if (GST_EVENT_TYPE (event) == GST_EVENT_FLUSH_START) {
> +    gst_segment_init (&self->segment, GST_FORMAT_UNDEFINED);
> 
> This should probably happen on flush stop (as in this case, the stream lock is
> held).

You mean that gst_segment_init() should be called for both events? FLUSH_START
and FLUSH_STOP?

> This function may also be a good case for a switch/case.

done.

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