[Bug 794911] Implement support for ULP Forward Error Correction

[GStreamer (GNOME Bugzilla)]

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

--- Comment #4 from Mathieu Duponchelle <mduponchelle1 at gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 370453 [details] [review]:
> 
> Are we always sending FEC now if requested on the media? Or only if the
> client (for PLAY) requests that from the SDP?
> 

So with this patch, only the RECORD case is implemented, and rtspclientsink
doesn't use the media API if that's what you're asking. In that case FEC is
only used on the client-side if it was requested through the new
ulpfec-percentage property of rtspclientsink, and on the server side we only
instantiate a decoder and tweak the storage properties if FEC was present in
the ANNOUNCE request.

I assume for PLAY, the client (rtspsrc) would not have a say on whether FEC is
used or not, it would simply be informed about it in the answer to the DESCRIBE
request.

Does that answer yyour question? :)

> ::: gst/rtsp-server/rtsp-media.c
> @@ +1519,3 @@
> +          i, &storage);
> +      if (storage)
> +        g_object_set (storage, "size-time", (media->priv->latency + 50) *
> GST_MSECOND, NULL);
> 
> Should this 50ms be configurable?
> 
> @@ +3132,3 @@
> +new_storage_cb (GstElement * rtpbin, GObject * storage, guint sessid,
> GstRTSPMedia * media)
> +{
> +  g_object_set (storage, "size-time", (media->priv->latency + 50) *
> GST_MSECOND, NULL);
> 
> And this 50ms?

This is a tad ugly, but no, this should not be configurable, we just want the
duration of the lookback window in the storage to be a tad higher than the
latency, otherwise by the time we get the PacketLost events in the ulpfec
decoder, the storage might have disposed of the relevant packets, 50 ms is
usually a safe value, though it hasn't been determined in an extremely
scientific manner :)

> 
> ::: gst/rtsp-server/rtsp-stream.c
> @@ +5019,3 @@
> +
> +
> +/**
> 
> Double new-line above
> 
> ::: gst/rtsp-server/rtsp-stream.h
> @@ +310,3 @@
> +/* ULP Forward Error Correction (RFC 5109) */
> +GST_RTSP_SERVER_API
> +gboolean           gst_rtsp_stream_get_ulpfec_enabled (GstRTSPStream
> *stream);
> 
> I wonder if some more generic API could be added here, similar to what is in
> rtpbin. So that we won't have to add yet another set of functions for
> flexfec later.

I wondered about that too, then opted for the less generic choice, because I'm
not really sure what the API will have to look like for flex, specifically
because of the fact multiple streams can be protected by the same flex
protection stream.

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