GNOME Bugzilla – Bug 795159
Various patches on ssrcdemux
Last modified: 2018-11-03 15:28:48 UTC
Created attachment 370796 [details] [review] rtpssrcdemux: introduce max-streams property The property is useful against attacks when the sender changes SSRC for every RTP packet. The property with the same name introduced in rtpbin was not enough, because we still can end up with thousands of pads allocated in rtpssrcdemux.
Created attachment 370797 [details] [review] rtpssrcdemux: Bad RTP/RTCP packet is not fatal When used for processing bundled media streams within rtpbin the rtpssrcdemux element may receive bad RTP and RTCP packets, these should not be treated as a fatal error.
Created attachment 370798 [details] [review] rtpssrcdemux: Handle RTCP APP packets Fix crash when processing RTCP APP packets.
Created attachment 370799 [details] [review] rtpssrcdemux: refactoring Avoid code-duplication and rename the lock to not appear as a native lock.
Review of attachment 370796 [details] [review]: ::: gst/rtpmanager/gstrtpssrcdemux.c @@ +284,3 @@ } + // We create 2 src pads per ssrc (RTP & RTCP). Checking if we are allowed + // to create 2 more pads Use /* */ style comments please @@ +285,3 @@ + // We create 2 src pads per ssrc (RTP & RTCP). Checking if we are allowed + // to create 2 more pads + num_streams = (GST_ELEMENT_CAST (demux)->numsrcpads) >> 1; Why ">>1" ? Do you want to do / 2 really ?
Review of attachment 370797 [details] [review]: Looks good
Review of attachment 370798 [details] [review]: Looks good. Shouldn't there be a default: case that doesn't crash too?
Review of attachment 370799 [details] [review]: Looks ok
(In reply to Olivier Crête from comment #4) > Review of attachment 370796 [details] [review] [review]: > > ::: gst/rtpmanager/gstrtpssrcdemux.c > @@ +284,3 @@ > } > + // We create 2 src pads per ssrc (RTP & RTCP). Checking if we are allowed > + // to create 2 more pads > > Use /* */ style comments please > > @@ +285,3 @@ > + // We create 2 src pads per ssrc (RTP & RTCP). Checking if we are allowed > + // to create 2 more pads > + num_streams = (GST_ELEMENT_CAST (demux)->numsrcpads) >> 1; > > Why ">>1" ? Do you want to do / 2 really ? Just that we get 2 pads for each stream, so this is a simple way of counting the number of streams I guess?
Ping?
Arg, the patches no longer apply :-(
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/461.