webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.h - Issue 3012243002: Change ForwardErrorCorrection class to accept one received packet at a time.

Side by Side Diff: webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.h

Issue 3012243002: Change ForwardErrorCorrection class to accept one received packet at a time. (Closed)

Patch Set: Fix compilation errors, including tests. Created 3 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

« webrtc/modules/rtp_rtcp/source/forward_error_correction.cc ('K') | « webrtc/modules/rtp_rtcp/source/rtp_fec_unittest.cc ('k') | webrtc/modules/rtp_rtcp/source/ulpfec_receiver_impl.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 /*	1 /*

2 * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.	2 * Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.

3 *	3 *

4 * Use of this source code is governed by a BSD-style license	4 * Use of this source code is governed by a BSD-style license

5 * that can be found in the LICENSE file in the root of the source	5 * that can be found in the LICENSE file in the root of the source

6 * tree. An additional intellectual property rights grant can be found	6 * tree. An additional intellectual property rights grant can be found

7 * in the file PATENTS. All contributing project authors may	7 * in the file PATENTS. All contributing project authors may

8 * be found in the AUTHORS file in the root of the source tree.	8 * be found in the AUTHORS file in the root of the source tree.

9 */	9 */

10	10

(...skipping 26 matching lines...) Expand all Loading...
37	37

38 private:	38 private:

39 const uint32_t ssrc_;	39 const uint32_t ssrc_;

40	40

41 rtc::CriticalSection crit_sect_;	41 rtc::CriticalSection crit_sect_;

42 RecoveredPacketReceiver* recovered_packet_callback_;	42 RecoveredPacketReceiver* recovered_packet_callback_;

43 std::unique_ptr<ForwardErrorCorrection> fec_;	43 std::unique_ptr<ForwardErrorCorrection> fec_;

44 // TODO(holmer): In the current version \|received_packets_\| is never more	44 // TODO(holmer): In the current version \|received_packets_\| is never more

45 // than one packet, since we process FEC every time a new packet	45 // than one packet, since we process FEC every time a new packet

46 // arrives. We should remove the list.	46 // arrives. We should remove the list.

47 ForwardErrorCorrection::ReceivedPacketList received_packets_;	47 // TODO(nisse): But it looks like

	48 // UlpfecReceiverImpl::AddReceivedRedPacket may add two packets to
	brandtr 2017/09/14 11:49:45 Right, I forgot about that! I guess it can be repl Right, I forgot about that! I guess it can be replaced by two calls to DecodeFec? I don't think we ever send RED packets with two payloads, and if we want to support receiving 2 payloads, why not support receiving N payloads? We just probably just try to get rid of Ulpfec at some point though. nisse-webrtc 2017/09/14 12:55:51 Things may get easier if we merge the Add...Packet Show quoted text On 2017/09/14 11:49:45, brandtr wrote: > Right, I forgot about that! I guess it can be replaced by two calls to > DecodeFec? Things may get easier if we merge the Add...Packet and Process...Fec methods both here and in flexfec. It seems callers always call them together. Show quoted text > I don't think we ever send RED packets with two payloads, and if we want to > support receiving 2 payloads, why not support receiving N payloads? No idea, I'd guess there's some interop case behind the choice to support 2? Show quoted text > We just probably just try to get rid of Ulpfec at some point though. But that's going to take a few years, I guess. brandtr 2017/09/14 13:14:15 In Flexfec, outside users call FlexfecReceiver::On Show quoted text On 2017/09/14 12:55:51, nisse-webrtc wrote: > On 2017/09/14 11:49:45, brandtr wrote: > > Right, I forgot about that! I guess it can be replaced by two calls to > > DecodeFec? > > Things may get easier if we merge the Add...Packet and Process...Fec methods > both here and in flexfec. It seems callers always call them together. In Flexfec, outside users call FlexfecReceiver::OnRtpPacket, which always call the two private methods together. https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/incl... In Ulpfec, outside users could call the different public methods separately, but they don't. https://cs.chromium.org/chromium/src/third_party/webrtc/modules/rtp_rtcp/sour... https://cs.chromium.org/chromium/src/third_party/webrtc/video/rtp_video_strea... Show quoted text > > I don't think we ever send RED packets with two payloads, and if we want to > > support receiving 2 payloads, why not support receiving N payloads? > > No idea, I'd guess there's some interop case behind the choice to support 2? Yes, probably. But why not support the N payload case then? Show quoted text > > We just probably just try to get rid of Ulpfec at some point though. > > But that's going to take a few years, I guess. Yes :(
	49 // the list.

	50 std::vector<std::unique_ptr<ForwardErrorCorrection::ReceivedPacket>>

	51 received_packets_;

48 ForwardErrorCorrection::RecoveredPacketList recovered_packets_;	52 ForwardErrorCorrection::RecoveredPacketList recovered_packets_;

49 FecPacketCounter packet_counter_;	53 FecPacketCounter packet_counter_;

50 };	54 };

51	55

52 } // namespace webrtc	56 } // namespace webrtc

53	57

54 #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_ULPFEC_RECEIVER_IMPL_H_	58 #endif // WEBRTC_MODULES_RTP_RTCP_SOURCE_ULPFEC_RECEIVER_IMPL_H_

OLD	NEW