Bug 215567 - calling transceiver setCodecPreferences doesn't change the order of codecs in the offer/answer generated by the browser
Summary: calling transceiver setCodecPreferences doesn't change the order of codecs in...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Safari Technology Preview
Hardware: Mac macOS 10.15
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-08-17 07:15 PDT by Jaya
Modified: 2020-09-03 10:32 PDT (History)
15 users (show)

See Also:


Attachments
Patch (6.75 KB, patch)
2020-09-02 04:00 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch (10.66 KB, patch)
2020-09-02 05:30 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jaya 2020-08-17 07:15:07 PDT
On STP 111, I am trying to change the default order of codecs in the offer/answer SDP by calling setCodecPreferences on the video transceiver.
It doesn't seem to have any effect on the order of the codecs in the subsequent offer/answer generated by the browser.
setCodecPreferences is working on Chrome 85 in Unified plan.

Steps to reproduce:
Modify https://webrtc.github.io/samples/src/content/peerconnection/pc1/ to include the following line before calling offer on pc1.

//localStream.getTracks().forEach(track => pc1.addT(track, localStream));
const audio = pc1.addTransceiver('audio', localStream.getAudioTracks()[0]);
const video = pc1.addTransceiver('video', localStream.getVideoTracks()[0]);
const capabilities = RTCRtpSender.getCapabilities('video').codecs;
  
capabilities.forEach((codec, idx) => {
  if (codec.mimeType === 'video/VP8') {
    capabilities.splice(idx, 1);
    capabilities.unshift(codec);
  }
});
video.setCodecPreferences(capabilities);
Comment 1 Radar WebKit Bug Importer 2020-08-17 16:03:24 PDT
<rdar://problem/67277554>
Comment 2 youenn fablet 2020-09-02 04:00:17 PDT
Created attachment 407754 [details]
Patch
Comment 3 youenn fablet 2020-09-02 05:30:59 PDT
Created attachment 407758 [details]
Patch
Comment 4 EWS Watchlist 2020-09-02 05:31:43 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 5 youenn fablet 2020-09-02 05:31:55 PDT
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/25360
Comment 6 EWS 2020-09-03 01:38:55 PDT
Committed r266508: <https://trac.webkit.org/changeset/266508>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407758 [details].
Comment 7 Darin Adler 2020-09-03 10:28:53 PDT
Comment on attachment 407758 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407758&action=review

> Source/WebCore/platform/mediastream/libwebrtc/LibWebRTCProvider.cpp:413
> +            if (hasParameter)
> +                sdpFmtpLine.append(";");
> +            else
> +                hasParameter = true;
> +            sdpFmtpLine.append(StringView(parameter.first.data(), parameter.first.length()));
> +            sdpFmtpLine.append("=");
> +            sdpFmtpLine.append(StringView(parameter.second.data(), parameter.second.length()));

Here’s a more efficient way to do this:

    const char* separator = hasParameter ? ";" : "";
    hasParameter = true;
    sdpFmtpLine.append(separator, StringView(parameter.first.data(), parameter.first.length()), '=', StringView(parameter.second.data(), parameter.second.length()));

A single call to append is more efficient than four separate calls to append.
Comment 8 Darin Adler 2020-09-03 10:32:35 PDT
Or if you prefer, like this:

    sdpFmtpLine.append(hasParameter ? ";" : "", StringView(parameter.first.data(), parameter.first.length()), '=', StringView(parameter.second.data(), parameter.second.length()));
    hasParameter = true;

Or even this:

    sdpFmtpLine.append(std::exchange(hasParameter, true) ? ";" : "", StringView(parameter.first.data(), parameter.first.length()), '=', StringView(parameter.second.data(), parameter.second.length()));