Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2867943003:
New class RtpDemuxer and RtpPacketSinkInterface, use in Call. (Closed)
|
Created:
3 years, 7 months ago by nisse-webrtc Modified:
3 years, 7 months ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tlegrand-webrtc, tterriberry_mozilla.com, mflodman, Taylor Brandstetter Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionNew class RtpDemuxer and RtpPacketSinkInterface, use in Call.
BUG=webrtc:7135
Review-Url: https://codereview.webrtc.org/2867943003
Cr-Commit-Position: refs/heads/master@{#18160}
Committed: https://chromium.googlesource.com/external/webrtc/+/e4bcd6d02aebdf61589b7e63985c984d20a84011
Patch Set 1 #Patch Set 2 : Fix use-after-free crash in av sync. #
Total comments: 24
Patch Set 3 : Address danil's comments. #
Total comments: 6
Patch Set 4 : Rebase, affects Call::OnRecoveredPacket. #Patch Set 5 : Let flexfec ignore recovered media packets, to fix infinite recursion. #Patch Set 6 : Comment improvements and nits. #Patch Set 7 : Fix Flexfec recursion issue in a different way. #
Total comments: 1
Messages
Total messages: 34 (13 generated)
nisse@webrtc.org changed reviewers: + brandtr@webrtc.org, danilchap@webrtc.org, pthatcher@webrtc.org, solenberg@webrtc.org
This cl adds a class RtpDemuxer, and the RtpPacketSinkInterface. Hopefully easier than https://codereview.webrtc.org/2709723003/ OnRecoveredPacket is broken in the first version. I see two approaches: 1. Keep the Call::video_receive_ssrcs_ and use it for demuxing of recovered packets. 2. Let Call::OnRecoveredPacket parse the packet (in the same way as DeliverRtp) and call video_rtp_demuxer_.OnRtpPacket with the packet. To be able to treat recovered packets separately in the receiver (not sure what differences there are), add a flag to RtpPacketReceived. I think I tried to do (2) some months ago, IIRC correctly one obstacle was that OnRecoveredPacket is part of two interfaces: RecoveredPacketReceiver and RtpData. And to pull this off, I suspect the best way is to retire RtpData (replaced by RtpPacketSinkInterface). Advice?
On 2017/05/09 07:34:44, nisse-webrtc wrote: > 2. Let Call::OnRecoveredPacket parse the packet (in the same way as DeliverRtp) > and call video_rtp_demuxer_.OnRtpPacket with the packet. To be able to treat > recovered packets separately in the receiver (not sure what differences there > are), add a flag to RtpPacketReceived. > > I think I tried to do (2) some months ago, Here's the cl, https://codereview.webrtc.org/2693123002/ I just rebased it, but I haven't yet tried to recall what it really was doing.
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (left): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#oldco... webrtc/call/call.cc:599: RTC_DCHECK(num_deleted == 1); may be move DCHECK that something was deleted inside RemoveSink function instead of removing it https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:205: // Audio, Video, and FlexFEC receive streams are owned by the client that Why merge flexfec into video? May be keep this CL just about introducing demuxer and leave merging discussin for later https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:217: RtpDemuxer audio_rtp_demuxer_; GUARDED_BY? https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:1201: // parse packets here, add a "recovered" flag to RtpPacketReceived, looks reasonable, may instead of adding flag 'recovered' rename 'retransmit' flag to 'recovered' (which was added exactly for this scenario but with less broad name) Then also need to ensure that flag is checked to avoid double-recovery. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.cc File webrtc/call/rtp_demuxer.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.c... webrtc/call/rtp_demuxer.cc:26: sinks_.insert(std::pair<uint32_t, RtpPacketSinkInterface*>(ssrc, sink)); sinks_.emplace(ssrc, sink); https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:32: virtual ~RtpDemuxer(); why this class has virtual functions?
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (left): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#oldco... webrtc/call/call.cc:599: RTC_DCHECK(num_deleted == 1); On 2017/05/09 09:20:22, danilchap wrote: > may be move DCHECK that something was deleted inside RemoveSink function instead > of removing it I can let RemoveSink return a deletion count, and then DCHECK that. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:205: // Audio, Video, and FlexFEC receive streams are owned by the client that On 2017/05/09 09:20:22, danilchap wrote: > Why merge flexfec into video? > May be keep this CL just about introducing demuxer and leave merging discussin > for later We currently support flexfec only on the video transport, and its naturally a sink for both its own packets and for the protected streams. Rasmus, do you agree? https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:217: RtpDemuxer audio_rtp_demuxer_; On 2017/05/09 09:20:22, danilchap wrote: > GUARDED_BY? Done. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:1201: // parse packets here, add a "recovered" flag to RtpPacketReceived, On 2017/05/09 09:20:22, danilchap wrote: > looks reasonable, > may instead of adding flag 'recovered' rename 'retransmit' flag to 'recovered' > (which was added exactly for this scenario but with less broad name) Sounds good to me, unless there's some reason to treat packets from rtx and fec separately? > Then also need to ensure that flag is checked to avoid double-recovery. I'm not following you. Where should it be checked? https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.cc File webrtc/call/rtp_demuxer.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.c... webrtc/call/rtp_demuxer.cc:26: sinks_.insert(std::pair<uint32_t, RtpPacketSinkInterface*>(ssrc, sink)); On 2017/05/09 09:20:22, danilchap wrote: > sinks_.emplace(ssrc, sink); Thanks, that looks better. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:32: virtual ~RtpDemuxer(); On 2017/05/09 09:20:22, danilchap wrote: > why this class has virtual functions? It's not needed for now, I'll remove virtual. (To me, non-virtual member functions are so alien to object oriented programming that I tend to not use that feature).
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:1201: // parse packets here, add a "recovered" flag to RtpPacketReceived, On 2017/05/09 12:11:48, nisse-webrtc wrote: > > Then also need to ensure that flag is checked to avoid double-recovery. > > I'm not following you. Where should it be checked? just a side comment, free to ignore it: If this flag is written here, some other code should read (check) it. https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.cc File webrtc/call/rtp_demuxer.cc (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.c... webrtc/call/rtp_demuxer.cc:32: count++; nit: ++count https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:218: RtpDemuxer video_rtp_demuxer_; The comment on RtpDemuxer said it's "per transport", but if we're bundling, then these two demuxers will share a transport. So it there a demuxer before these demuxers which sorts out audio vs video? And what about when we will (as part of WebRTC 1.0) have more than one video transport of audio transport (the completely unbundled case)? Then we'd need more than one audio demuxer and more than one video demuxer. Perhaps it's best to say that our demuxer is currently per-media-type right now and put a TODO to change it to be per transport. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:26: // This class represents the RTP demuxing, for a single transport. It We should define "transport" here. I think you mean it in the RTP sense of "one SSRC space", but it should be made clear. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:35: virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); What happens if a single SSRC has multiple sinks added? What happens if you give a null sink?
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:218: RtpDemuxer video_rtp_demuxer_; On 2017/05/10 00:04:21, pthatcher1 wrote: > The comment on RtpDemuxer said it's "per transport", but if we're bundling, then > these two demuxers will share a transport. So it there a demuxer before these > demuxers which sorts out audio vs video? Currently there's a payload demuxer somewhere splitting into audio and video. My intention is to move over to a single demuxer for the bundled case. The demuxer should be owned by RtpTransportController, owned by PeerConnection. Call will get two RtpTransportController pointers, one for audio and one for video, and in the bundled case, both pointers will refer to the same object. > And what about when we will (as part > of WebRTC 1.0) have more than one video transport of audio transport (the > completely unbundled case)? Then we'd need more than one audio demuxer and more > than one video demuxer. That's a case I haven't considered. To me, the tricky question is, when creating a new media stream, e.g., Call::CreateVideoReceiveStream, how do we know which transport (and demuxer) the stream should use? https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:26: // This class represents the RTP demuxing, for a single transport. It On 2017/05/10 00:04:21, pthatcher1 wrote: > We should define "transport" here. I think you mean it in the RTP sense of "one > SSRC space", but it should be made clear. I mean one SSRC space, and my understanding is that corresponds to one transport, which is admittedly little fuzzy, but for a direct connection that would be one 5-tuple, and with ice it would be one "ice session", I guess. Can you suggest a concise and unambiguous term for that? https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:35: virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); On 2017/05/10 00:04:21, pthatcher1 wrote: > What happens if a single SSRC has multiple sinks added? What happens if you > give a null sink? We can have multiple sinks for the same ssrc, e.g, a media stream and flexfex receiver for a "flexfec group". Received packets are passed on to all sinks registered for that ssrc. We can also have the same sink registered on multiple ssrcs, e.g, the flexfec case above, and also rtx. Although the latter should maybe be refactored at some point to use a separate rtx receiver. Use of null sinks, and repeated pairs of {ssrc, sink}, is not intended. Speaking of flexfec, I and danil were a bit unsure of the flexfec scope. I'm working under the assumption that a flexfec group can't span multiple transports. I.e., each flexfec receiver belongs to one transport and it can protect only media streams on that same transport. Correct?
deadbeef@webrtc.org changed reviewers: + deadbeef@webrtc.org
I only skimmed over the Call code, but this looks good to me. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:26: // This class represents the RTP demuxing, for a single transport. It On 2017/05/10 07:41:32, nisse-webrtc wrote: > On 2017/05/10 00:04:21, pthatcher1 wrote: > > We should define "transport" here. I think you mean it in the RTP sense of > "one > > SSRC space", but it should be made clear. > > I mean one SSRC space, and my understanding is that corresponds to one > transport, which is admittedly little fuzzy, but for a direct connection that > would be one 5-tuple, and with ice it would be one "ice session", I guess. > > Can you suggest a concise and unambiguous term for that? RTP session? From RFC7656: "An RTP session shares a single SSRC space" "A single media transport always carries a single RTP session." https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/call.cc#newco... webrtc/call/call.cc:1204: // and then just pass it on to video_rtp_demuxer_.OnRtpPacket? That makes sense. Or just have an "OnRecoveredRtpPacket" method on RtpDemuxer. https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:28: // to the user of this class. Could you mention in a comment that this should also eventually handle payload type and MID-based demuxing? Payload type based demuxing is currently handled in BaseChannel, which we're refactoring out into RtpTransport. Which hopefully in the future will deliver packets directly to RtpDemuxer.
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc File webrtc/call/call.cc (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/call.cc#newco... webrtc/call/call.cc:218: RtpDemuxer video_rtp_demuxer_; On 2017/05/10 07:41:32, nisse-webrtc wrote: > On 2017/05/10 00:04:21, pthatcher1 wrote: > > The comment on RtpDemuxer said it's "per transport", but if we're bundling, > then > > these two demuxers will share a transport. So it there a demuxer before these > > demuxers which sorts out audio vs video? > > Currently there's a payload demuxer somewhere splitting into audio and video. My > intention is to move over to a single demuxer for the bundled case. The demuxer > should be owned by RtpTransportController, owned by PeerConnection. Call will > get two RtpTransportController pointers, one for audio and one for video, and in > the bundled case, both pointers will refer to the same object. > Can you please document this, then? Because just saying it's "per-transport" does not make this clear. > > And what about when we will (as part > > of WebRTC 1.0) have more than one video transport of audio transport (the > > completely unbundled case)? Then we'd need more than one audio demuxer and > more > > than one video demuxer. > > That's a case I haven't considered. To me, the tricky question is, when creating > a new media stream, e.g., Call::CreateVideoReceiveStream, how do we know which > transport (and demuxer) the stream should use? That is, indeed, a difficult question. In PeerConnection, that means we will have N RtpTransports. And if an RtpTransport is tied to something down here, then we need N things down here as well. Perhaps this gets a big TODO for now and we have a discussion with Taylor (who is leading this work in the PeerConnection) about this. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:26: // This class represents the RTP demuxing, for a single transport. It On 2017/05/10 20:59:43, Taylor Brandstetter wrote: > On 2017/05/10 07:41:32, nisse-webrtc wrote: > > On 2017/05/10 00:04:21, pthatcher1 wrote: > > > We should define "transport" here. I think you mean it in the RTP sense of > > "one > > > SSRC space", but it should be made clear. > > > > I mean one SSRC space, and my understanding is that corresponds to one > > transport, which is admittedly little fuzzy, but for a direct connection that > > would be one 5-tuple, and with ice it would be one "ice session", I guess. > > > > Can you suggest a concise and unambiguous term for that? > > RTP session? From RFC7656: > > "An RTP session shares a single SSRC space" > "A single media transport always carries a single RTP session." That's exactly what I meant. Basically quote/reference/summarize that here. https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:35: virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); On 2017/05/10 07:41:32, nisse-webrtc wrote: > On 2017/05/10 00:04:21, pthatcher1 wrote: > > What happens if a single SSRC has multiple sinks added? What happens if you > > give a null sink? > > We can have multiple sinks for the same ssrc, e.g, a media stream and flexfex > receiver for a "flexfec group". Received packets are passed on to all sinks > registered for that ssrc. > Could you make that clear in the comment? > We can also have the same sink registered on multiple ssrcs, e.g, the flexfec > case above, and also rtx. Although the latter should maybe be refactored at some > point to use a separate rtx receiver. Yes, that was clear from the comment. > > Use of null sinks, and repeated pairs of {ssrc, sink}, is not intended. > I think we need to document what happens if a null or duplicate is passed in. Does it crash? Does it return an error? Does it just log a warning and ignore you? > Speaking of flexfec, I and danil were a bit unsure of the flexfec scope. I'm > working under the assumption that a flexfec group can't span multiple > transports. I.e., each flexfec receiver belongs to one transport and it can > protect only media streams on that same transport. Correct? It's my understanding that flexfec will not span multiple SSRC spaces (else how could it reference two spaces that have the same SSRC?). So, yes, I believe that's correct.
I got an interesting error after rebase. Problem is that when flexfec recovers a packet, it's passed to Call::OnRecoveredPacket, and now I pass it to the same demuxer as regular media packets. Now, since flexfec is registered as a sink, packet is passed back to flexfec. And somehow the flexfec logic ends up calling OnRecoveredPacket again, and we get a nice infinite recursion... So we'd need to have flexfec ignore incoming packets with the recovered() flag set. I've added a check in FlexfecReceiver::AddReceivedPacket, to see if it solves the problem, but I don't think that's quite the right place. I imagine flexfec can also get packets which are retransmitted, and those shouldn't be ignored. To avoid DOS issues, I imagine we also need some sane handling for the unexpected case that a recovered packet happens to be a flexfec packet, rather than a media packet. Rasmus, what's the right way to handle this?
https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/20001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:35: virtual void AddSink(uint32_t ssrc, RtpPacketSinkInterface* sink); On 2017/05/12 00:38:51, pthatcher1 wrote: > On 2017/05/10 07:41:32, nisse-webrtc wrote: > > On 2017/05/10 00:04:21, pthatcher1 wrote: > > > What happens if a single SSRC has multiple sinks added? What happens if you > > > give a null sink? > > > > We can have multiple sinks for the same ssrc, e.g, a media stream and flexfex > > receiver for a "flexfec group". Received packets are passed on to all sinks > > registered for that ssrc. > > > > Could you make that clear in the comment? > > > We can also have the same sink registered on multiple ssrcs, e.g, the flexfec > > case above, and also rtx. Although the latter should maybe be refactored at > some > > point to use a separate rtx receiver. > > Yes, that was clear from the comment. > > > > > Use of null sinks, and repeated pairs of {ssrc, sink}, is not intended. > > > > I think we need to document what happens if a null or duplicate is passed in. > Does it crash? Does it return an error? Does it just log a warning and ignore > you? I'm updating the comments to say that null pointers are not allowed, and I'm adding DCHECKs to back that up. For repeated pairs of {ssrc, sink}, I don't think anything special happens, one will just end up with duplicate entries in the underlying multimap. https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.cc File webrtc/call/rtp_demuxer.cc (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.c... webrtc/call/rtp_demuxer.cc:32: count++; On 2017/05/09 13:23:41, danilchap wrote: > nit: ++count > https://google.github.io/styleguide/cppguide.html#Preincrement_and_Predecrement Done. https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:28: // to the user of this class. On 2017/05/10 20:59:43, Taylor Brandstetter wrote: > Could you mention in a comment that this should also eventually handle payload > type and MID-based demuxing? Payload type based demuxing is currently handled in > BaseChannel, which we're refactoring out into RtpTransport. Which hopefully in > the future will deliver packets directly to RtpDemuxer. I'm adding a TODO. Have I got this right, that payload type and MID demuxing is relevant only for unsignalled ssrcs?
https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h File webrtc/call/rtp_demuxer.h (right): https://codereview.webrtc.org/2867943003/diff/40001/webrtc/call/rtp_demuxer.h... webrtc/call/rtp_demuxer.h:28: // to the user of this class. On 2017/05/12 08:50:08, nisse-webrtc wrote: > On 2017/05/10 20:59:43, Taylor Brandstetter wrote: > > Could you mention in a comment that this should also eventually handle payload > > type and MID-based demuxing? Payload type based demuxing is currently handled > in > > BaseChannel, which we're refactoring out into RtpTransport. Which hopefully in > > the future will deliver packets directly to RtpDemuxer. > > I'm adding a TODO. Have I got this right, that payload type and MID demuxing is > relevant only for unsignalled ssrcs? Correct. Though JSEP no longer requires SSRC signaling, so this is something we'll likely have to deal with soon.
lgtm
lgtm
lgtm. Looks like the mac_rel failure is a flake: Interrupted: ./rtc_unittests AsyncInvokeTest.CancelInvoker (try #1) https://codereview.webrtc.org/2867943003/diff/120001/webrtc/modules/rtp_rtcp/... File webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc (right): https://codereview.webrtc.org/2867943003/diff/120001/webrtc/modules/rtp_rtcp/... webrtc/modules/rtp_rtcp/source/flexfec_receiver.cc:135: // Set this flag first, since OnRecoveredPacket may end up here I'll look into why this is the case.
The CQ bit was checked by nisse@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by nisse@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from danilchap@webrtc.org, pthatcher@webrtc.org Link to the patchset: https://codereview.webrtc.org/2867943003/#ps120001 (title: "Fix Flexfec recursion issue in a different way.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17059)
On 2017/05/16 08:58:00, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > presubmit on master.tryserver.webrtc (JOB_FAILED, > http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17059) lgtm for audio_receive_stream.h
stefan@webrtc.org changed reviewers: + stefan@webrtc.org
Nice improvement. LGTM
The CQ bit was checked by nisse@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1494935068904890,
"parent_rev": "7be9e42f6c870004a5aa9b123b9f28c8f95f5b88", "commit_rev":
"e4bcd6d02aebdf61589b7e63985c984d20a84011"}
Message was sent while issue was closed.
Description was changed from ========== New class RtpDemuxer and RtpPacketSinkInterface, use in Call. BUG=webrtc:7135 ========== to ========== New class RtpDemuxer and RtpPacketSinkInterface, use in Call. BUG=webrtc:7135 Review-Url: https://codereview.webrtc.org/2867943003 Cr-Commit-Position: refs/heads/master@{#18160} Committed: https://chromium.googlesource.com/external/webrtc/+/e4bcd6d02aebdf61589b7e639... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:120001) as https://chromium.googlesource.com/external/webrtc/+/e4bcd6d02aebdf61589b7e639... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
