[Bug 794911] Implement support for ULP Forward Error Correction

[GStreamer (GNOME Bugzilla)]

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

--- Comment #5 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Mathieu Duponchelle from comment #4)
> (In reply to Sebastian Dröge (slomo) from comment #3)
> > Review of attachment 370453 [details] [review] [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? :)

Yeah, does that work for PLAY already then? Probably not as we don't add the
relevant pieces to the SDP?


> > ::: 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 :)

For RTX this is configurable though, it's the same thing there right?

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

Ok then :)

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