671 Commits

Author SHA1 Message Date
Philipp Hancke
7233df9751 Filter fmtp parameters from RED capabilities
and ensure there is only one, similar to what is done with RTX.
This avoids exposing a payload type there.

See also
  https://github.com/w3c/webrtc-pc/issues/2696

BUG=webrtc:42221750,webrtc:360058654

Change-Id: Id7c2ddeaf47a3169db9be43c9c5b8e59346f1d57
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376760
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43929}
2025-02-19 09:43:31 -08:00
Evan Shrubsole
f052c432fe Move string_to_number.h to webrtc namespace
Bug: webrtc:42232595
Change-Id: I104cff12bf40509fb4554b98f7af16975263285a
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/377520
Commit-Queue: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43911}
2025-02-18 01:44:39 -08:00
Henrik Boström
fe25b0e928 Report 'outbound-rtp.targetBitrate' correctly and per-RTP stream.
This CL fixes two issues with the old way targetBitrate was reported:
1. The target is per encoder, i.e. per SSRC, but the old way to report
   it was per sender and was approximately the sum of all encodings'
   targetBitrate in most cases.
2. The old value did not come directly from the VideoBitrateAllocation
   and tended to be greater than the sum of all targets (don't know
   why).

We know the old value was wrong and the new value correct because
the actual bytes produced by the encoder closely matches the configured
target, which wasn't always the case with the old metric implementation.

Tested with unit tests and manually in Chrome by going to
https://henbos.github.io/codec-quality/src/index.html and ensuring
target ~= actual bytes produced. It also matches the debug logging of
video_stream_encoder.cc.

Bug: webrtc:42225524, chromium:392424845
Change-Id: I7a6f69e053ebc3fd972c2c4b7712750e721c0acc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376460
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43854}
2025-02-06 09:55:11 -08:00
Henrik Boström
ede69fd577 Make IsSameRtpCodecIgnoringLevel work for any codec.
Prior to this CL, IsSameRtpCodecIgnoringLevel() only ignored level IDs
if the codec was H265, incorrectly considering, for example, different
levels of H264 Baseline as not equal.
- This CL fixes that problem by using IsSameCodecSpecific() which is
  already used in other places, reducing the risk of different
  comparisons using different comparison rules.

This also fixes https://crbug.com/webrtc/391340599 where
setParameters() would throw if unrecognized SDP FMTP parameters were
added to a codec as part of SDP negotiation via SDP munging.

This CL makes the following WPT tests pass:
- external/wpt/webrtc/protocol/h264-unidirectional-codec-offer.https.html
- fast/peerconnection/RTCRtpSender-setParameters.html

Bug: chromium:381407888, webrtc:391340599
Change-Id: I5991403b56c86ba97e670996c6687f6315dde304
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374043
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43797}
2025-01-24 05:37:17 -08:00
Henrik Boström
860a13c6fd Misc improvements to RtpTransceiver unit tests and test utils.
In order to reduce the size and scope of a follow-up CL, this CL makes
some cleaning up and improvements to existing tests and adds some minor
test utility methods that will be used in the follow-up.

No change in behavior, this CL...
- Makes use of NiceMock in RtpTransceiver tests to avoid wall of text
  spam for various "uninteresting" method calls in all tests in this
  file.
- Refactors creating senders, receivers and transceivers to allow the
  follow-up CL to create such objects for kind "video" as well.
- Exposes cricket::FakeVideoEngine* to RtpTranscieverTest and allows
  adding unidirectional video codecs in the fake engine, to be used by
  the follow-up CL's tests.
- Allows creating fake video engine codecs from SdpVideoFormat in the
  fake decoder factory (already possible in the fake encoder factory).

Bug: chromium:381407888
Change-Id: Ie07eff79d832dd21800b95fd584891ebf4520798
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374900
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43776}
2025-01-20 23:55:17 -08:00
Henrik Boström
9f68535e68 Fix setParameters() throwing when level-id does not match.
In order to align with this PR[1], setParameters() should not throw if
the H265 level ID we're trying to send does not match what was
negotiated. This was believed to be fixed by [2] but we were still
throwing due to a check on a different layer (media_engine.cc).

In order to reproduce the issue despite WebRTC lacking SW
encoder/decoder for H265, peer_connection_encodings_integrationtest.cc
gets a new test with real stack but fake encoder/decoder factory. This
allows negotiating H265 and doing SetParameters() even though the codec
is not processing any frames.
- Basic test coverage is added for singlecast and simulcast H265.
- Test coverage for the bug being fixed added.
- In Chrome the equivalent WPTs exists for when real HW is available
  here[3]. Those tests PASS with this CL (currently FAIL).

[1] https://github.com/w3c/webrtc-pc/pull/3023
[2] https://webrtc-review.googlesource.com/c/src/+/368781
[3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webrtc/protocol/h265-level-id.https.html

Bug: chromium:381407888
Change-Id: I3619a124586b8b26d3695cfad8890cf40bd475db
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374164
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Jianlin Qiu <jianlin.qiu@intel.com>
Cr-Commit-Position: refs/heads/main@{#43759}
2025-01-17 09:17:11 -08:00
Sergey Silkin
a65c453f9e Reduce default max QP for AV1 from 56 to 52
Before this CL VP8 and AV1 used the same max QP=56. Tests show that at this QP AV1 delivers a worse PSNR than VP8. We want AV1 min quality to be not worse than VP8. This CL reduces the default max QP for AV1 to 52. With this value libaom AV1 encoder delivers PSNR close to libvpx VP8 at QP 56.

Bug: webrtc:351644568, b/369540380
Change-Id: I2e27ddab562f9c9710b11dc09076b03d7b308bb0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374041
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43751}
2025-01-16 10:55:53 -08:00
Philipp Hancke
cfaba8fd2d Measure SDP munging
by storing
  [[LastCreatedOffer]] / [[LastCreatedAnswer]]
which are similar to the W3C equivalent but as
description objects instead of serialized SDP strings.

While rejecting all SDP munging is not feasible, this lets us
measure and reject certain modifications gradually.

Chromium metrics CL:
  https://chromium-review.googlesource.com/c/chromium/src/+/6089633

This is measured at three points during the lifetime of a peerconnection:
* for the first SLD call
* when the connection is first established
* when the connection was established and is being closed

Note that the "first" SDP munging detected is returned which may hide that something uses more than one modification.

BUG=chromium:40567530

Change-Id: I964e3ee6e75f73b777d90556fac8691a6f3dc27f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370680
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Johannes Kron <kron@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43741}
2025-01-15 07:38:45 -08:00
Boris Tsirkin
256d828aac Format /media folder
Formatting done via:

git ls-files | grep -E '^media\/.*\.(h|cc|mm)' | xargs clang-format -i

No-Iwyu: Includes didn't change and it isn't related to formatting
Bug: webrtc:42225392
Change-Id: I7c79f10d8ac2c2e3a9f796d2ca3c542b9826683c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/373701
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43685}
2025-01-08 09:51:41 -08:00
Per Kjellander
776866774f Propagate desicion if RTP packet should be ECT(1) marked to socket
With this CL, the decision if an RTP packet should be sent as ect(1) is made in RtpControllerSend depending on if RFC 8888 has been negotiated and if CCFB is received with ECN enabled.
Since webrtc does not yet adapt to ECN feedback, packets are sent as ECT(1) until the first feedback is received.

Change-Id: Iddf63849328afbe54a7c8f921f2e8db134aeff6a
Bug: webrtc:42225697
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367388
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43609}
2024-12-19 01:59:49 -08:00
Harald Alvestrand
9f827f51f8 Extend FindMatchingCodec to support multi-codec RED
This was exercised by a test, but multi-codec RED is not currently
generated by WebRTC.
RED spec allows it, so failing in comparator seems wrong.
This was one of the cases where the referenced bug was triggered,
but not the only one.

Bug: webrtc:384756621
Change-Id: I28c101aa34a62083b72b5f7fc12d25fc637db209
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372060
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43607}
2024-12-18 23:33:31 -08:00
Harald Alvestrand
33f38f2f38 Add some tests for CodecList consistency
Bug: webrtc:360058654
Change-Id: Ida26eca237c4f882cf03204a3d87780c25c1890c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371640
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43604}
2024-12-18 17:34:32 -08:00
Harald Alvestrand
486e3deba0 Expand use of CodecList to cover pc/media_session.cc
This allows us to verify consistency of codec lists in more places.

Bug: webrtc:360058654
Change-Id: Ibd0d10579c4b8058031db0df458e8fc9e2181152
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371921
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43594}
2024-12-18 01:41:51 -08:00
Harald Alvestrand
611d7f610c Introduce the CodecList class
Lists of codecs have a lot of cross references (RTX/APT and the like).
We should introduce functionality to verify that those linkages are correct
before modifying the handling of these.

This CL introduces the CodecList class, which can be extended to do
that verification. It is used by pc/media_session.cc, but inter-module
APIs are not changed in this version (they will be later).

Bug: webrtc:360058654
Change-Id: Ifd6313d0289cfa090e51ac28bc775265d18fe6f2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/371600
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43582}
2024-12-16 14:15:21 -08:00
Harald Alvestrand
882b32d00f Reland "Use PayloadTypePicker for video PT assignment"
This reverts commit e046787a5a80a9d292b3aec7e946644e025a2b95.

Reason for revert: Revised codec matching to fix issue.

Changes also back out some changes that should not have been
included (using PayloadTypePicker for codec list merging).

Original change's description:
> Revert "Use PayloadTypePicker for video PT assignment"
>
> This reverts commit e5048949b0fcc275264e24f3b2a4c658fcc84aa3.
>
> Reason for revert: Broke internal tests.
>
> Original change's description:
> > Use PayloadTypePicker for video PT assignment
> >
> > This includes changes that change the order of codecs.
> > It is preparatory to doing late assignment of video PTs.
> >
> > Bug: webrtc:360058654
> > Change-Id: Id5ddaf94d4b9557c0502a373e42635108d8fdf26
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366400
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#43489}
>
> Bug: webrtc:360058654
> Change-Id: I5c94a7bafa49bdf17f665480398707155e458d26
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370240
> Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43490}

Bug: webrtc:360058654
Change-Id: I66b3b6bd657c66f8860c5e67a504266d7707f48d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370380
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Artem Titov <titovartem@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43554}
2024-12-12 16:37:30 -08:00
Harald Alvestrand
e046787a5a Revert "Use PayloadTypePicker for video PT assignment"
This reverts commit e5048949b0fcc275264e24f3b2a4c658fcc84aa3.

Reason for revert: Broke internal tests.

Original change's description:
> Use PayloadTypePicker for video PT assignment
>
> This includes changes that change the order of codecs.
> It is preparatory to doing late assignment of video PTs.
>
> Bug: webrtc:360058654
> Change-Id: Id5ddaf94d4b9557c0502a373e42635108d8fdf26
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366400
> Reviewed-by: Henrik Boström <hbos@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43489}

Bug: webrtc:360058654
Change-Id: I5c94a7bafa49bdf17f665480398707155e458d26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370240
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43490}
2024-12-03 22:24:21 +00:00
Harald Alvestrand
e5048949b0 Use PayloadTypePicker for video PT assignment
This includes changes that change the order of codecs.
It is preparatory to doing late assignment of video PTs.

Bug: webrtc:360058654
Change-Id: Id5ddaf94d4b9557c0502a373e42635108d8fdf26
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366400
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43489}
2024-12-03 18:18:28 +00:00
Tommi
98b3588974 Make CreateSendChannel and CreateReceiveChannel methods pure virtual
These methods previously had a default implementation that triggered
a crash. All implementations must now return a valid object, which
simplifies the code that calls them.

Bug: webrtc:13931
Change-Id: I877fbc929b58c6b83767c6ac5a81c8aa942e3fef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369021
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43453}
2024-11-26 09:17:35 +00:00
Tomas Lundqvist
b40c559858 Set voice RTCP mode based on the RemoteContent and not based on the LocalContent.
The RTCP mode is a send property for both send and receive channels. Send properties should be configured based on what peers support/prefer, which is described by the remote description (content).


Bug: webrtc:340041654
Change-Id: I18cd59e98aecfbbd8f4919b98381836184c10d77
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368980
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Tomas Lundqvist <tomasl@google.com>
Cr-Commit-Position: refs/heads/main@{#43449}
2024-11-25 14:06:39 +00:00
Harald Alvestrand
2e7e049bb4 Don't use transport-cc if RFC8888 feedback is negotiated.
Bug: webrtc:378698658
Change-Id: I06536445d32577b7b4d24ae7ca529d9b270b34d0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368863
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43435}
2024-11-21 18:15:05 +00:00
Qiu Jianlin
5ad1daeed9 setParameters should not throw when only level mismatch.
According to latest requirement, when the level reported by
RtpSender.getCapabilities() for H.265 is different from that was
negotiated, we should not throw when setParameters() is called with
level-id set to that reported by RtpSender.getCapabilities().
Underlingly negotiated codec level should remain unchanged.

Bug: chromium:41480904
Change-Id: I28bbdb5f0a0ab0d98315f56c80004601afc91a63
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368781
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43434}
2024-11-21 09:20:12 +00:00
Qiu Jianlin
2d47c9395b Correct H.265 level-id in fmtp line for offer/answer.
On a sendrecv m-line, the offered level-id represents the maximum that
can be both sent and received; on a sendonly m-line, the offered
level-id represents the maximum that can be sent; on a recvonly m-line,
the offered level-id represents the maximum that can be received.
Also according to RFC 7798 section 5, the highest level indicated by the
answer is either equal to or lower than that in the offer

Bug: chromium:41480904
Change-Id: I1729c8edc3aed0c00c41cea96204abafc37c002b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367322
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Jianlin Qiu <jianlin.qiu@intel.com>
Cr-Commit-Position: refs/heads/main@{#43425}
2024-11-19 13:09:13 +00:00
Harald Alvestrand
752235261e Remove all references to codec-level transport-cc functions and flags.
This seems to have no effect on tests, so it appears that these were
not used after all.
The goal is to make transport-cc a media-section-level attribute.

Bug: webrtc:378698658
Change-Id: Ia20ca5b91472b02db30f911ad1a1892cf36cd682
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368440
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43411}
2024-11-18 10:20:01 +00:00
Evan Shrubsole
7589689774 Replace cricket::LeastCommonMultiple and cricket::GreatestCommonDivisor with std::lcm and std::gcd.
The std::lcm and std::gcd functions are part of the C++ standard
library. The existing functions are marked as deprecated rather than
deleted in the case of possible third party uses.

#rtc_cleanup

Bug: webrtc:377205743
Change-Id: I174e663f152d750c984a35dc7136bc18dc01bc8e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367440
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Auto-Submit: Evan Shrubsole <eshr@google.com>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43368}
2024-11-07 13:30:28 +00:00
Dor Hen
3fa21c89c0 Comment unused variables in implemented functions 13\n
Increased the number of errors the automation is fixing to 150 from
75 in this commit.

Bug: webrtc:370878648
Change-Id: If6e6a5f40db7eb54c27c1a85fb7031838e478c70
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366205
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Cr-Commit-Position: refs/heads/main@{#43337}
2024-10-31 07:44:25 +00:00
Dor Hen
297fe1a2d9 [reland] Comment unused variables in implemented functions 10\n
Bug: webrtc:370878648
Change-Id: Icbfacf8113942f60ba168e4aa884f0172eaa0fca
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367080
Commit-Queue: Dor Hen <dorhen@meta.com>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43336}
2024-10-31 07:08:00 +00:00
Harald Alvestrand
461e828d57 Revert "Comment unused variables in implemented functions 10\n"
This reverts commit f5e0f038440ae1cf46f84c3d740a75e420d808ca.

Reason for revert: Use of [[maybe_unused]] in .h files compiled in objC

Original change's description:
> Comment unused variables in implemented functions 10\n
>
> Bug: webrtc:370878648
> Change-Id: Ic2dda55058ed4474d898fa938c2a66dab2f6f20e
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366204
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Commit-Queue: Dor Hen <dorhen@meta.com>
> Cr-Commit-Position: refs/heads/main@{#43327}

Bug: webrtc:370878648, b/376178831
Change-Id: Ibeaecd6ae21b6fc478ce153ad72f8941d7af4a46
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/367060
Reviewed-by: Christoffer Dewerin <jansson@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#43331}
2024-10-30 08:22:53 +00:00
Dor Hen
c118881416 Comment unused variables in implemented functions 12\n
Bug: webrtc:370878648
Change-Id: Ia9b1db4f6c393a016c3769cd57c540704e9ca4f7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366526
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43329}
2024-10-29 17:26:38 +00:00
Dor Hen
a154b73097 Comment unused variables in implemented functions 11\n
Bug: webrtc:370878648
Change-Id: Ic31d7744cc8516e4c014bc044fbe2dba9e4d835b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366525
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Cr-Commit-Position: refs/heads/main@{#43328}
2024-10-29 17:25:36 +00:00
Dor Hen
f5e0f03844 Comment unused variables in implemented functions 10\n
Bug: webrtc:370878648
Change-Id: Ic2dda55058ed4474d898fa938c2a66dab2f6f20e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366204
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Cr-Commit-Position: refs/heads/main@{#43327}
2024-10-29 17:24:33 +00:00
Jakob Ivarsson
7058da6e29 Rename PacketOptions.is_retransmit to is_media.
It is used to distinguish between audio/video packets and everything else (retransmit/padding/fec), so naming it is_media makes more sense.

This is a follow up to https://webrtc-review.googlesource.com/366644

Bug: b/375148360
Change-Id: Ia53f4d707ceb85f059688d86bc5dcc2d57908d88
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366424
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43319}
2024-10-28 13:45:23 +00:00
Jakob Ivarsson
68f4e27794 Add RtpSender OnFirstPacketSent callback.
It works in the same way as the first packet received callback and can be used for latency measurements.

One important detail is that RTCP and probe packets are excluded from triggering the callback.

Bug: b/375148360
Change-Id: I5f99b565f96b622e864669cf227be5534aab0fc7
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366644
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Jakob Ivarsson‎ <jakobi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43309}
2024-10-25 16:17:04 +00:00
Henrik Boström
e8c97c0d09 Reland "Rename requested_resolution to scale_resolution_down_to."
This is a reland of commit 82617ac51e7825db53451818f4d1ad52b69761fd

The reason for the revert was a downstream use of
`rtc::VideoSinkWants::requested_resolution`, so in this reland we don't
rename this field, it's fine just to rename the one in
RtpEncodingParameters for now.

Original change's description:
> Rename `requested_resolution` to `scale_resolution_down_to`.
>
> This is a pure refactor/rename CL without any changes in behavior.
>
> This field is called scaleResolutionDownTo in the spec and JavaScript.
> Let's make C++ match to avoid confusion.
>
> In order not to break downstream during the transition a variable with
> the old name being a pure reference to the renamed attribute is added.
> This means we have to add custom constructors, but we can change this
> back to "= default" when the transition is completed, which should only
> be a couple of CLs away.
>
> Bug: webrtc:375048799
> Change-Id: If755102ccd79d46020ce5b33acd3dfcc29799e47
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366560
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43300}

NOTRY=True

Bug: webrtc:375048799
Change-Id: Ic4ee156c1d50aa36070a8d84059870791dcbbe5e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366660
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43304}
2024-10-25 08:39:49 +00:00
Florent Castelli
af44d8ff06 Revert "Rename requested_resolution to scale_resolution_down_to."
This reverts commit 82617ac51e7825db53451818f4d1ad52b69761fd.

Reason for revert: Break downstream projects

Original change's description:
> Rename `requested_resolution` to `scale_resolution_down_to`.
>
> This is a pure refactor/rename CL without any changes in behavior.
>
> This field is called scaleResolutionDownTo in the spec and JavaScript.
> Let's make C++ match to avoid confusion.
>
> In order not to break downstream during the transition a variable with
> the old name being a pure reference to the renamed attribute is added.
> This means we have to add custom constructors, but we can change this
> back to "= default" when the transition is completed, which should only
> be a couple of CLs away.
>
> Bug: webrtc:375048799
> Change-Id: If755102ccd79d46020ce5b33acd3dfcc29799e47
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366560
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43300}

Bug: webrtc:375048799
Change-Id: Ie41723a39420e12e7b5b681d3d00ccd14f66b4b1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366642
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#43301}
2024-10-24 14:51:29 +00:00
Henrik Boström
82617ac51e Rename requested_resolution to scale_resolution_down_to.
This is a pure refactor/rename CL without any changes in behavior.

This field is called scaleResolutionDownTo in the spec and JavaScript.
Let's make C++ match to avoid confusion.

In order not to break downstream during the transition a variable with
the old name being a pure reference to the renamed attribute is added.
This means we have to add custom constructors, but we can change this
back to "= default" when the transition is completed, which should only
be a couple of CLs away.

Bug: webrtc:375048799
Change-Id: If755102ccd79d46020ce5b33acd3dfcc29799e47
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366560
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43300}
2024-10-24 11:38:21 +00:00
Henrik Boström
1c262bf5a4 Allow not specifying requested_resolution on inactive encodings.
This fixes the bug where scaleResolutionDownTo must be specified even
on inactive encodings (scaleResolutionDownTo is the JavaScript name for
what is called requested_resolution inside WebRTC).

Bug: chromium:375048792
Change-Id: I3206ef7de09eaba24a5b4305d888ec4904617e58
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366522
Auto-Submit: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43292}
2024-10-23 12:37:14 +00:00
Harald Alvestrand
b7abaee819 Revert "Use Payload Type suggester for all codec merging"
This reverts commit 0bac2aae596771db020f01a57fee4828081fbc38.

Reason for revert: Suspected breakages downstream

Original change's description:
> Use Payload Type suggester for all codec merging
>
> Bug: webrtc:360058654
> Change-Id: Id475762253c427c1800c2352a60fc0121c2dc388
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364783
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43267}

Bug: webrtc:360058654, b/375132036
Change-Id: Ieda626270193e7e6c93903b3c03a691b2bf0c1e2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366540
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43290}
2024-10-23 11:37:18 +00:00
Harald Alvestrand
7a3c07b8ef Cleanup: Move all comparator tests to codec_comparators_unittests
This CL has no functional changes.

Bug: webrtc:360058654
Change-Id: I28a9347a5787efd068bc207d4ab72d27cf7400c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/366202
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Auto-Submit: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43277}
2024-10-22 11:52:15 +00:00
Åsa Persson
929c02a479 Add IsSameRtpCodec method to Codec.
This is similar to MatchesRtpCodec but not an exact match of parameters, unspecified parameters are treated as default. Use IsSameRtpCodec for comparison when codec is configured via encodings.

Bug: b:299588022
Change-Id: I0ea800e50af6f5666e3e867a928e15b0aa044635
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365142
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Åsa Persson <asapersson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43272}
2024-10-21 11:25:24 +00:00
Harald Alvestrand
0bac2aae59 Use Payload Type suggester for all codec merging
Bug: webrtc:360058654
Change-Id: Id475762253c427c1800c2352a60fc0121c2dc388
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364783
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43267}
2024-10-18 16:58:42 +00:00
Dor Hen
049b43bd02 [reland] Comment unused variables in implemented functions
Compiling webrtc with `-Werror=unused-parameters` is failling duo to
those parameters.
Also, it shouldn't harm us to put those in comment for code readability as
well.

NOTE: This time I made sure to iterate over the C files in the
audio_processing folder and compile them using gcc.
On the original CL that was reverted - that failed with the same error
Danil mentioned. This time it seems fine.
I'll make sure to run the same script on the rest of my CLs for sanity

Bug: webrtc:370878648
Change-Id: I83cea3a08777e21d26a95bcad503a2d1b74566eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364537
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Dor Hen <dorhen@meta.com>
Cr-Commit-Position: refs/heads/main@{#43249}
2024-10-16 11:40:33 +00:00
Olov Brändström
558c2dc539 Change timestamps type from int64 to Timestamp in MediaReceiverInfo.
We should use the Timestamp type, rather then int64, to store timestamps. In https://webrtc-review.googlesource.com/c/src/+/365001/ an additional int64 timestamp was added (last_sender_report_timestamp_ms).

This CL fixes the new timestamp, as well as other similar timestamps in MediaReceiverInfo (last_sender_report_utc_timestamp_ms and last_sender_report_remote_utc_timestamp_ms).

Bug: webrtc:372393493
Change-Id: I0e473730e85a69ec595b421e2c3db920364008eb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365641
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Olov Brändström <brandstrom@google.com>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43248}
2024-10-16 11:02:37 +00:00
Harald Alvestrand
bd42ee8750 Refactor FindMatchingCodec
This is in preparation for making a matcher that checks the parameters
when all payload types come from the same number space.

Bug: webrtc:360058654
Change-Id: Ibcf4fee8d882eb0fa7f83faf0278bc6757761e18
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365361
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43223}
2024-10-11 10:52:43 +00:00
Harald Alvestrand
ae40039522 Add comparators unittest, and abandon MatchesForSdp
Use the same code in PayloadTypePicker as in Codec.Matches()

Bug: webrtc:360058654
Change-Id: I549ed24860648cfdb6a173a19773daf01db827b0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365102
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43217}
2024-10-10 14:33:13 +00:00
Harald Alvestrand
3203b626f4 Add AbslStringify for cricket::Codec
This makes debug output easier to read.

Bug: webrtc:360058654
Change-Id: I887be638489cde26868db0db2950262255213160
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365144
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43216}
2024-10-10 14:30:34 +00:00
Olov Brändström
b74268e0cf Update TODOs to the correct format.
Some TODOs that where added in https://webrtc-review.googlesource.com/c/src/+/365001 do not follow the correct format
https://webrtc.googlesource.com/src/+/refs/heads/main/g3doc/style-guide.md#comments. This CL updates the incorrect TODOs.

Also updated some comments as they referred to ntp timestamps, when the timestamp is utc.

Bug: None
Change-Id: I1661f6f57c9fa5f66e5b92f154007c34854923c6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365162
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Olov Brändström <brandstrom@google.com>
Cr-Commit-Position: refs/heads/main@{#43214}
2024-10-10 11:21:44 +00:00
Harald Alvestrand
19bbd6f02f Move some codec-comparing functions to a single file.
This CL is a pure move; later CLs will try to increase consistency
between the functions.

Bug: webrtc:360058654
Change-Id: I6662b3d35f8e2dab60c2778a4755454fe3029fe2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365100
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43210}
2024-10-09 22:10:36 +00:00
Olov Brändström
fea3280c13 update format of recently added TODOs.
Some TODOs with an old from where added in https://webrtc-review.googlesource.com/c/src/+/363946.

This CL updates the TODO comments to the current form.

Bug: None
Change-Id: Id61dca5a0f4d705f4dfe74f6523dae3e357d49ba
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365140
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Olov Brändström <brandstrom@google.com>
Cr-Commit-Position: refs/heads/main@{#43209}
2024-10-09 18:34:03 +00:00
Olov Brändström
51b682648e Add an environment clock timestamp to SenderReportStats.
Add an environment clock timestamp to SenderReportStats and make it visible in rtc_stats_collector.cc. This make it possible to use the pc->GetConfiguration().stats_timestamp_with_environment_clock() flag to decide which timestamp to use when creating a RTCRemoteOutboundRtpStreamStats object.

This CL is the third (and possible the last) of a series of CLs that aim to replace the UTC timestamps in RTCStats objects to Environment clock timestamps. The other CLs where https://webrtc-review.googlesource.com/c/src/+/363946 and https://webrtc-review.googlesource.com/c/src/+/364782.

When Chromium and Google internal uses of RTCStats are updated to set the stats_timestamp_with_environment_clock configuration, the flag can be deleted.

Bug: chromium:369369568
Change-Id: Ic0b07d7b012505267bd6516f19a9ba90df4cafab
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365001
Reviewed-by: Henrik Andreassson <henrika@webrtc.org>
Commit-Queue: Olov Brändström <brandstrom@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43206}
2024-10-09 12:59:08 +00:00
Olov Brändström
b9c4c242d4 rename timestamps to show epoch
I missed one timestamp in https://webrtc-review.googlesource.com/c/src/+/363946, meaning that the config flag that was added do not yet work for all timestamps in RTCStats objects. The RTCRemoteOutboundRtpStreamStats still has UTC timestamps even if the config flag is set.

I will solve this by saving both an UTC (existing) and env (to be added) timestamp, and then let rtc_stats_collector choose timestamp based on the value of the config flag (just like RTCRemoteInboundRtpStreamStats is done in the 363946 commit).

Before adding the new env_ timestamp I want to make this change. I rename the existing timestamp to show what epoch it uses (NTP or UTC). This will later make it clear which timestamp is which.

So this CL will make no logical change, just renaming members.

I only need to rename the last_sender_report_timestamp_ms, but opted to rename the remote timestamp as well, to be consistent with the naming convention I add in this CL.

Bug: chromium:369369568
Change-Id: Icfe7cf274995b39799e1478a1bb8cdf5134f0b16
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364782
Commit-Queue: Olov Brändström <brandstrom@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43194}
2024-10-08 16:27:58 +00:00