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}
Webrtc is build with FFmpeg sources on defined in the include path
through the -I flag, so they should be included this way instead. This
would otherwise cause a conflict when the chromium ffmpeg sources move
from third_party/ffmpeg/* to third_party/ffmpeg/src/*
BUG: chromium:329282834
Change-Id: Id8f7e91446bdc536db77e74388a73e51f5111ffc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342820
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Ted (Chromium) Meyer <tmathmeyer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#41899}
Before this change, calling buffered_amount only included what was
buffered on top of what was already buffered in the SCTP socket. With
the defaults, the SCTP socket can buffer up to 2MB of data (that is not
put on the wire) before the additional external bufferering in
SctpDataChannel will be used. The buffering that I am working on
removing completely.
Until it's removed completely, to avoid the issue reported in
crbug.com/41221056, include the bytes buffered in the SCTP socket to
what is returned when calling RTCDataChannel::buffered_amount.
This means that when this value is zero, it can be safe to know that all
bytes have been sent, but not necessarily acknowledged. And calling
close will not discard any messages.
This is a stopgap solution, but as functional as the proper solution
that removes all additional buffering. Follow-up CLs will merely improve
this solution.
Bug: chromium:41221056
Change-Id: I06edd52188d3bf13a17827381a15a4730722685a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342520
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41898}
This seems to confuse perfetto, and the data ends up on its own track
and the end event is just ignored. As it was invalid, I am assuming it
is not used, and can be simply removed.
#rtc_fixit
Bug: webrtc:15867
Change-Id: I31a814f6c2147c3ce534726bf9046a79369b9eb3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/342761
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Evan Shrubsole <eshr@google.com>
Cr-Commit-Position: refs/heads/main@{#41896}
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}