GNOME Bugzilla – Bug 747208
rtpvp8depay: should have width/height in its caps so it can be fed to muxers
Last modified: 2015-04-14 01:41:25 UTC
this pipeline fails, it shouldn't: udpsrc ! rtpvp8depay ! matroskamux ! fakesink The reason it fails is that rtpvp8depay doesn't parse the width/height out of the VP8 stream and we don't have a vp8parse. A vp8parse was rejected in bug #637831t
Fixed: commit d410acf64981420c6102535c1c9ab5d5581870c8 Author: Olivier Crête <olivier.crete@collabora.com> Date: Wed Apr 1 19:30:27 2015 -0400 rtpvp8depay: Parse width/height/profile from keyframes This makes it possible to mux the result into a container such as matroska. https://bugzilla.gnome.org/show_bug.cgi?id=747208
This commit seems to break in the case that somehow or other the intra frame is dropped (say, in the network) as it blindly drops all packets until the next keyframe comes, but it makes no attempt to request a keyframe if any are dropped.
It drops all buffers before the first keyframe, not subsequent ones. The reason is that we don't want to push out buffers before the caps, but we can't get the caps without a keyframe. And the VP8 spec specifies that a valid VP8 stream must start with a keyframe anyway. But it possibly makes sense to emit a GstForceKeyUnit event when that happens?
Created attachment 301094 [details] [review] rtpvp8depay: When dropping intra packet, request keyframe
(In reply to Robert Swain from comment #2) > This commit seems to break in the case that somehow or other the intra frame > is dropped (say, in the network) as it blindly drops all packets until the > next keyframe comes, but it makes no attempt to request a keyframe if any > are dropped. Note that this IMHO is also an OpenWebRTC bug. It sends stuff too early, so that the first keyframe always gets dropped. See my patchset to improve on that :) Nonetheless Olivier's patch, and the new one make sense to me and should get in.
I wonder if nicesink shouldn't somehow be hooked to emit a keyunit request when it gets to the connected state for the first time. Currently Farstream does that. I should also mention that libnice has a bug where it doesn't drop packets from "unknown" hosts, those which haven't yet returned a reply from the ICE request, so that may result in more dropped packets when it is fixed, as it seems to be a security feature for WebRTC.
(In reply to Sebastian Dröge (slomo) from comment #5) > (In reply to Robert Swain from comment #2) > > This commit seems to break in the case that somehow or other the intra frame > > is dropped (say, in the network) as it blindly drops all packets until the > > next keyframe comes, but it makes no attempt to request a keyframe if any > > are dropped. > > Note that this IMHO is also an OpenWebRTC bug. It sends stuff too early, so > that the first keyframe always gets dropped. See my patchset to improve on > that :) That just makes it easier to trigger. It is entirely possible that those packets could be dropped anyway and therefore this is a valid problem. :) > Nonetheless Olivier's patch, and the new one make sense to me and should get > in. +1
(In reply to Olivier Crête from comment #6) > I wonder if nicesink shouldn't somehow be hooked to emit a keyunit request > when it gets to the connected state for the first time. Currently Farstream > does that. I should also mention that libnice has a bug where it doesn't > drop packets from "unknown" hosts, those which haven't yet returned a reply > from the ICE request, so that may result in more dropped packets when it is > fixed, as it seems to be a security feature for WebRTC. DTLS handshaking needs to have completed as well.
(In reply to Olivier Crête from comment #6) > I wonder if nicesink shouldn't somehow be hooked to emit a keyunit request > when it gets to the connected state for the first time. But it should only really do that if the first buffer after it is connected is not a keyframe. Just mentioning that for completeness, you probably thought of that already ;) Also I don't think it's the right thing to do in every situation, but more an application policy thing
I think the only sensible thing for rtpvp8depay to do here is to request keyframes when it gets into this situation. As from the outside it's impossible to know that something is wrong (before the depayloader we don't have indications of what is a keyframe and what not, and the depayloader just silently drops things). Maybe request a keyframe once every 5 seconds if none arrived yet, based on the RTP timestamps. Other opinions? Seems like rtptheoradepay also does something like that, but it actually seems to request a new keyframe for every single broken packet.
Or as Rob said on IRC, maybe just request a new keyframe for every packet... and throttle via a property in rtpsession to not spam the sender.
rtpsession should already be throttled to the RTCP sending rate anyway. If no keyframe comes, it should be sending a PLI at every interval. The idea was indeed that the rest of the pipeline an spam rtpsession with key unit requests. Merged my patch: commit 1394a66e62532492b2c43456c8d68cd35591d272 Author: Olivier Crête <olivier.crete@collabora.com> Date: Tue Apr 7 18:00:53 2015 -0400 rtpvp8depay: When dropping intra packet, request keyframe https://bugzilla.gnome.org/show_bug.cgi?id=747208
Should that have said 'When dropping inter packets before keyframe...' ?