RtpPacketToSend::transport_sequence_number
packed_id is set to be 64 bit to align with rtc::PacketOptions.
packet_id is only set to RtpPacketToSend::transport_sequence_number if
TransportSequenceNumber header extension is not used in order to not
change current behaviour.
Bug: webrtc:15368
Change-Id: Ia532714226421422bdb292f8dd34b175560e9dc6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344160
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41950}
And move writing of the header extension from PacketRouter to
RtpSenderEgress::SendPacket.
Bug: webrtc:15368
Change-Id: Ieb18af4bc20115bf02d37e1f9a815a5c120975a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343786
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41949}
Start migrating away from `hasAudioLevel`, `voiceActivity`, `audioLevel` fields in RTPHeaderExtension and switch usages to a more modern absl::optional<AudioLevel> accessor instead.
The old fields are preserved for compatibility with downstream projects, but will be removed in the future.
Bug: webrtc:15788
Change-Id: I76599124fd68dd4d449f850df3b9814d6a002f5d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/336303
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41947}
Try to increase the timeout to see if it solves the flakiness issue.
If it doesn't work we should temporary disable this test.
Change-Id: I8ecf3721cb5f7f4c647c8cbf247740c89c72ab82
Bug: webrtc:15885
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343982
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Jeremy Leconte <jleconte@google.com>
Cr-Commit-Position: refs/heads/main@{#41946}
This CL removes the send buffers (but not the receive buffer) from
SctpDataChannel and increases the send buffer in DcSctpSocket instead.
The reasons are:
1) Simplify the code. This additional buffering was strictly needed
before we migrated away from usrsctp, as that send buffer was very
limited in size (by design). But with the migration to dcSCTP, it's
no longer needed, so it just adds complexity.
2) Make `RTCDataChannel::bufferedAmount` correct. Before this CL, it
represented just the data buffered in SctpDataChannel, and not the
data accepted by the SCTP socket, but not yet put on the wire. This
makes it hard for clients to know when a message has ever been sent.
3) Better handle draining data on data channel close. While this is not
implemented in dcSCTP, having a single buffer makes this easier to
add.
While most of this CL is straightforward, the handling of bufferedAmount
in the signaling thread (in RTCDataChannel in Blink), is a bit special.
The number returned by `RTCDataChannel::bufferedAmount` is not what the
true value is inside the SCTP socket, but an eventual consistent view
of that value. When a message is sent, the value is incremented and:
- Before this change: When a message was put on the SCTP socket, the
view's value was decremented. Which made the view reflect what was
buffered outside the SCTP socket, and that buffering is now gone.
- After this change: SctpDataChannel will track what RTCDataChannel
will think it is, and provide updates to that number as we are
notified that it's reduced - by setting a "low threshold" callback
trigger.
A bonus with the new behavior is that it will be eventually consistent
and auto-heal also in error conditions - when messages are dropped due
to errors (bad input, bad state, etc). Previously, the bufferedAmount
value could drift away from the correct value on errors.
Note that a big chunk of unit tests were removed with this CL, as those
tested how the buffering behaved. Now, there is no buffering, so the
removed test cases represent a simpler interface.
This CL has been extensively tested with data channel benchmarks that
use the bufferedAmount thresholds (in Javascript).
Bug: chromium:40072842
Change-Id: I1a6a4af6b6e1116832f5028f989ce9f44683d229
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343361
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41945}
This code was extracted to make the next following CL easier to review.
This CL simply exposes the getters, setters and callbacks to set the
buffered amount low threshold on a specific SCTP stream. It will be
used in a follow-up CL, but is just boilerplate.
Bug: chromium:40072842
Change-Id: Iccd72208b369ddc252cc5886f6446b9c2ceeb0b1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343360
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41943}
The parse error testcase creates a random byte string
and tries to parse it as a delta attribute expecting it to fail.
Ubsan detected that there was "unsafe" static_cast<>, where
a value from network is static_casted:ed into a enum.
That enum was then *checked* for validity, so I think it was
same before aswell.
This fix changes to do the check/convering as one step.
Bug: webrtc:15392
Change-Id: Ie2534deef8988bc3c3179e194155cfd48b0ee6e5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343980
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41942}
This reverts commit c39712b51522bb21c18c58c593f454c5cc0e7995.
Reason for revert: Fixed issue where frame rate not adapted to highest "active" requested frame rate.
Patchset 1 contains original cl.
Later patchsets contains modifications.
Original change's description:
> Propagate known Encoder SinkWants when configured instead of after first frame.
>
> Propagate requested resolution and max frame rate to the source when
> configured rather than after the first frame.
> This is so that the source can be configured immediately. There is no
> reason why source should be updated until after first frame since it may lead
> to unnecessary reconfigurations and thread jumps. Wants that depend on actual frame size is not moved.
>
> Cl also change default behaviour in VideoStreamEncoderTest to not
> set restriction on max frame rate.
>
Bug: webrtc:14451
Change-Id: I2668db44bd17586efcf511ad3cd975065c503ec5
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343122
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41941}
If SVC is used, the minimum bitrate would be 30kbps, instead of 49, as
configured in svc_config.h, because the overall stream will get min_bitrate
from the default VP8 simulcast configuration, and this 30kbps will be
allocated to the stream for svc_rate_allocator to divide between layers.
However, with the configuration before this change, 49kbps would be always
allocated to the lowest simulcast stream.
Bug: webrtc:15852
Change-Id: I1c77f45654af8850180a83f8e3f4428cc42d086e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343760
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41940}
As it is currently implemented, the VP9 depacketizer decides packet's frame type based on p_bit ("Inter-picture predicted layer frame"). p_bit is set to 0 for upper spatial layer frames of keyframe since they do not have temporal refs. This results in marking packets of upper spatial layer frames, and, eventually these frames, of SVC keyframes as "keyframe" while they are in fact delta frames.
Normally spatial layer frames are merged into a superframe and the superframe is passed to decoder. But passing individual layers to a single decoder instance is a valid scenario too and is used in downstream projects. In this case, an upper layer frame marked as keyframe may cause decoder reset [2] and break decoding.
This CL changes frame type decision logic in the VP9 depacketizer such that only packets with both P and D (inter-layer predicted) bits unset are considered as keyframe packets.
When spatial layer frames are merged into a superframe in CombineAndDeleteFrames [1], frame type of the superframe is inferred from the lowest spatial layer frame.
[1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/modules/video_coding/frame_helpers.cc;l=53
[2] https://source.corp.google.com/piper///depot/google3/third_party/webrtc/files/stable/webrtc/modules/video_coding/codecs/vp9/libvpx_vp9_decoder.cc;l=209
Bug: webrtc:15827
Change-Id: Idc3445636f0eae0192dac998876fedec48628560
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343342
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41939}
FFmpeg has removed this field and usage of it in chromium must be
removed before the ffmpeg dependency is updated. The chromium media
change can be found here:
https://chromium-review.googlesource.com/c/chromium/src/+/5384308
The usage of the field in webrtc seems only to be for sanity checking,
so it should be just safe to remove entirely, since webrtc does not
expect re-ordering at all.
Bug: chromium:330573128
Change-Id: I9c5854ec82c3ad2d55374ea4eaa0c571437f8267
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343840
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#41935}
Without this, if max allocated bitrate is lowered while exponential probing is ongoing, a new probe can be sent at a rate of the new low max allocated bitrate which may cause BWE to decrease.
Bug: webrtc:14928
Change-Id: Id8e314740c2403d3b801d28f634dbc718f77c16e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343384
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41929}
This restricts the interface such that only a single onclose handler
can be set and that only one OnClose() notification will be fired.
That behavior is the same as how the previous sigslot was being
used, but the difference is that, in addition to removing sigslot,
this pattern is now more explicitly checked in the design.
Bug: webrtc:11943
Change-Id: I469c8cab3d62544988c8145b326af60b06b76d8e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343340
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41920}
The class is now only used by one test class.
Bug: none
Change-Id: Ib7714469254bd507d027385d2825b1c14bd63c94
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343123
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41917}
TRACE_EVENT is already scoped!
#rtc_fixit
Tested: Compiled the patch in Chromium and confirmed the Proxy events are still present. I can send the resulting trace to reviewers if desired.
Bug: webrtc:15867
Change-Id: I5717a85c0ee25e8e20123afa08064c9b6666ba96
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342920
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/main@{#41916}
This reverts commit 1ee24a650c116509d855e2ed23b8d28a0bb37384.
Reason for revert: Suspected upstream test breakage.
Original change's description:
> Propagate known Encoder SinkWants when configured instead of after first frame.
>
> Propagate requested resolution and max frame rate to the source when
> configured rather than after the first frame.
> This is so that the source can be configured immediately. There is no
> reason why source should be updated until after first frame since it may lead
> to unnecessary reconfigurations and thread jumps. Wants that depend on actual frame size is not moved.
>
> Cl also change default behaviour in VideoStreamEncoderTest to not
> set restriction on max frame rate. This aligns with how its used.
>
> Bug: webrtc:14451
> Change-Id: I96a3675d3ccabb1d2ecb4354b6932bc6563b1760
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342801
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41906}
Bug: webrtc:14451
Change-Id: I3aa669f8cc61a43b0820a06edf1497f3c86e3958
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343220
Auto-Submit: Per Kjellander <perkj@webrtc.org>
Owners-Override: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41911}
And thus require Environment to be propagated to this test helper
Bug: webrtc:15860
Change-Id: Ia4796d7a6a8e6f5dcb947899617df43e991419e7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343181
Reviewed-by: Jeremy Leconte <jleconte@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41910}
Keeping the old setting for the total queue size
limit, which avoids breaking a downstream.
This reverts commit 47ce449afaf9ba38785437fdd338630cad24a77b
and relands commit 4c990e2e56157175324e651f95f3d8c6a0e5c030.
Bug: chromium:40072842
Change-Id: I1e7d14b5d0026232d1fc9277172b6947b8be3490
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343120
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41907}
Propagate requested resolution and max frame rate to the source when
configured rather than after the first frame.
This is so that the source can be configured immediately. There is no
reason why source should be updated until after first frame since it may lead
to unnecessary reconfigurations and thread jumps. Wants that depend on actual frame size is not moved.
Cl also change default behaviour in VideoStreamEncoderTest to not
set restriction on max frame rate. This aligns with how its used.
Bug: webrtc:14451
Change-Id: I96a3675d3ccabb1d2ecb4354b6932bc6563b1760
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342801
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41906}
When an error occurs, the callback needs to be invoked or the
signaling thread may block indefinitely waiting for it.
Bug: webrtc:15871
Change-Id: Ib73382aff07b3632794300985223c70c24f554f4
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342901
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41904}
This reverts commit 4c990e2e56157175324e651f95f3d8c6a0e5c030.
Reason for revert: Breaks downstream build.
Original change's description:
> dcsctp: Add per-stream-limit, refactor limits.
>
> The limits have been moved out from the Send Queue as they were enforced
> outside the queue anyway (in the socket). That was a preparation for
> adding even more limits; There is now also a per-stream limit, allowing
> individual streams to have one (global) limit, and the entire socket to
> have another limit.
>
> These limits are very small in the default options. In Chrome, the limit
> is 16MB per stream, so expect the defaults to be updated when the
> additional buffering outside dcSCTP is removed.
>
> Bug: chromium:41221056
> Change-Id: I9f835be05d349cbfce3e9235d34b5ea0e2fe87d1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342481
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Commit-Queue: Victor Boivie <boivie@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#41895}
Bug: chromium:41221056
Change-Id: Icd57fbfca87d6b512cfc7f7682ae709000c2bcad
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343080
Commit-Queue: Björn Terelius <terelius@webrtc.org>
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#41901}
