Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix setCodecPreferences not working #78

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

netsy
Copy link
Contributor

@netsy netsy commented Apr 25, 2023

The bug introduces the wrong value of channels for video codecs when calling getRtpSenderCapabilities() or getRtpReceiverCapabilities(). The default value for channels should be -1.

The bug then prevents setCodecPreferences() from being applied since the channels does not match the original one.
https://github.com/flutter-webrtc/flutter-webrtc/blob/main/common/cpp/src/flutter_peerconnection.cc#L796

void FlutterPeerConnection::RtpTransceiverSetCodecPreferences()

    auto codecNumChannels = findInt(codecMap, "channels");

    if (codecNumChannels != -1)
      codecCapability->set_channels(codecNumChannels);
RTCError VerifyCodecPreferences(const std::vector<RtpCodecCapability>& codecs,
                                const std::vector<T>& send_codecs,
                                const std::vector<T>& recv_codecs) {
  // If the intersection between codecs and
  // RTCRtpSender.getCapabilities(kind).codecs or the intersection between
  // codecs and RTCRtpReceiver.getCapabilities(kind).codecs only contains RTX,
  // RED or FEC codecs or is an empty set, throw InvalidModificationError.
  // This ensures that we always have something to offer, regardless of
  // transceiver.direction.

  if (!absl::c_any_of(codecs, [&recv_codecs](const RtpCodecCapability& codec) {
        return codec.name != cricket::kRtxCodecName &&
               codec.name != cricket::kRedCodecName &&
               codec.name != cricket::kFlexfecCodecName &&
               absl::c_any_of(recv_codecs, [&codec](const T& recv_codec) {
                 return recv_codec.MatchesCapability(codec);
               });
      })) {
    return RTCError(RTCErrorType::INVALID_MODIFICATION,
                    "Invalid codec preferences: Missing codec from recv "
                    "codec capabilities.");
  }
bool Codec::MatchesCapability(
    const webrtc::RtpCodecCapability& codec_capability) const {
  webrtc::RtpCodecParameters codec_parameters = ToCodecParameters();

  return codec_parameters.name == codec_capability.name &&
         codec_parameters.kind == codec_capability.kind &&
         (codec_parameters.name == cricket::kRtxCodecName ||
          (codec_parameters.num_channels == codec_capability.num_channels &&
           codec_parameters.clock_rate == codec_capability.clock_rate &&
           codec_parameters.parameters == codec_capability.parameters));
}

The bug introduces the wrong value of `channels` for video codecs when calling getRtpSenderCapabilities() or getRtpReceiverCapabilities().
Copy link
Member

@cloudwebrtc cloudwebrtc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@cloudwebrtc cloudwebrtc merged commit ec7892e into webrtc-sdk:master Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants