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

Rename "announced ip" to "announced address" #1324

Merged
merged 13 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
### NEXT

- Make transport-cc feedback work similarly to libwebrtc ([PR #1088](https://github.com/versatica/mediasoup/pull/1088) by @penguinol).
- `TransportListenInfo`: announced ip can also be a hostname ([PR #1322](https://github.com/versatica/mediasoup/pull/1322)).
- `TransportListenInfo`: "announced ip" can also be a hostname ([PR #1322](https://github.com/versatica/mediasoup/pull/1322)).
- `TransportListenInfo`: Rename "announced ip" to "announced address" ([PR #1323](https://github.com/versatica/mediasoup/pull/1323)).

### 3.13.17

Expand Down
8 changes: 4 additions & 4 deletions node/src/Router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ export class Router<
listenInfo = {
protocol: 'udp',
ip: listenIp.ip,
announcedIp: listenIp.announcedIp,
announcedAddress: listenIp.announcedIp,
port: port,
};
}
Expand Down Expand Up @@ -896,7 +896,7 @@ export class Router<
? FbsTransportProtocol.UDP
: FbsTransportProtocol.TCP,
listenInfo!.ip,
listenInfo!.announcedIp,
listenInfo!.announcedAddress,
listenInfo!.port,
socketFlagsToFbs(listenInfo!.flags),
listenInfo!.sendBufferSize,
Expand Down Expand Up @@ -1162,12 +1162,12 @@ export class Router<
.then(() => {
return Promise.all([
localPipeTransport.connect({
ip: remotePipeTransport.tuple.localIp,
ip: remotePipeTransport.tuple.localAddress,
port: remotePipeTransport.tuple.localPort,
srtpParameters: remotePipeTransport.srtpParameters,
}),
remotePipeTransport.connect({
ip: localPipeTransport.tuple.localIp,
ip: localPipeTransport.tuple.localAddress,
port: localPipeTransport.tuple.localPort,
srtpParameters: localPipeTransport.srtpParameters,
}),
Expand Down
14 changes: 13 additions & 1 deletion node/src/Transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,19 @@ export type TransportListenInfo = {
ip: string;

/**
* @deprecated Use |announcedAddress| instead.
*
* Announced IPv4, IPv6 or hostname (useful when running mediasoup behind NAT
* with private IP).
*/
announcedIp?: string;

/**
* Announced IPv4, IPv6 or hostname (useful when running mediasoup behind NAT
* with private IP).
*/
announcedAddress?: string;

/**
* Listening port.
*/
Expand Down Expand Up @@ -136,7 +144,9 @@ export type TransportSocketFlags = {
};

export type TransportTuple = {
// @deprecated Use localAddress instead.
localIp: string;
localAddress: string;
localPort: number;
remoteIp?: string;
remotePort?: number;
Expand Down Expand Up @@ -1354,7 +1364,9 @@ export function serializeProtocol(

export function parseTuple(binary: FbsTransport.Tuple): TransportTuple {
return {
localIp: binary.localIp()!,
// @deprecated Use localAddress instead.
localIp: binary.localAddress()!,
localAddress: binary.localAddress()!,
localPort: binary.localPort(),
remoteIp: binary.remoteIp() ?? undefined,
remotePort: binary.remotePort(),
Expand Down
5 changes: 4 additions & 1 deletion node/src/WebRtcTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ export type IceParameters = {
export type IceCandidate = {
foundation: string;
priority: number;
// @deprecated Use |address| instead.
ip: string;
address: string;
protocol: TransportProtocol;
port: number;
type: IceCandidateType;
Expand Down Expand Up @@ -864,7 +866,8 @@ function parseIceCandidate(
return {
foundation: binary.foundation()!,
priority: binary.priority(),
ip: binary.ip()!,
ip: binary.address()!,
address: binary.address()!,
protocol: parseProtocol(binary.protocol()),
port: binary.port(),
type: iceCandidateTypeFromFbs(binary.type()),
Expand Down
8 changes: 8 additions & 0 deletions node/src/test/test-PlainTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ test('router.createPlainTransport() succeeds', async () => {
expect(plainTransport2.closed).toBe(false);
expect(plainTransport2.appData).toEqual({ foo: 'bar' });
expect(typeof plainTransport2.tuple).toBe('object');
// @deprecated Use tuple.localAddress instead.
expect(plainTransport2.tuple.localIp).toBe('9.9.9.1');
expect(plainTransport2.tuple.localAddress).toBe('9.9.9.1');
expect(typeof plainTransport2.tuple.localPort).toBe('number');
expect(plainTransport2.tuple.protocol).toBe('udp');
expect(plainTransport2.rtcpTuple).toBeUndefined();
Expand Down Expand Up @@ -138,11 +140,15 @@ test('router.createPlainTransport() succeeds', async () => {
expect(transport2.closed).toBe(false);
expect(transport2.appData).toEqual({});
expect(typeof transport2.tuple).toBe('object');
// @deprecated Use tuple.localAddress instead.
expect(transport2.tuple.localIp).toBe('127.0.0.1');
expect(transport2.tuple.localAddress).toBe('127.0.0.1');
expect(transport2.tuple.localPort).toBe(rtpPort);
expect(transport2.tuple.protocol).toBe('udp');
expect(typeof transport2.rtcpTuple).toBe('object');
// @deprecated Use tuple.localAddress instead.
expect(transport2.rtcpTuple?.localIp).toBe('127.0.0.1');
expect(transport2.rtcpTuple?.localAddress).toBe('127.0.0.1');
expect(transport2.rtcpTuple?.localPort).toBe(rtcpPort);
expect(transport2.rtcpTuple?.protocol).toBe('udp');
expect(transport2.sctpParameters).toBeUndefined();
Expand Down Expand Up @@ -395,7 +401,9 @@ test('plainTransport.getStats() succeeds', async () => {
expect(stats[0].probationBytesSent).toBe(0);
expect(stats[0].probationSendBitrate).toBe(0);
expect(typeof stats[0].tuple).toBe('object');
// @deprecated Use tuple.localAddress instead.
expect(stats[0].tuple.localIp).toBe('127.0.0.1');
expect(stats[0].tuple.localAddress).toBe('127.0.0.1');
expect(typeof stats[0].tuple.localPort).toBe('number');
expect(stats[0].tuple.protocol).toBe('udp');
expect(stats[0].rtcpTuple).toBeUndefined();
Expand Down
4 changes: 3 additions & 1 deletion node/src/test/test-WebRtcTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,9 @@ test('WebRtcTransport events succeed', async () => {

const onIceSelectedTuple = jest.fn();
const iceSelectedTuple: TransportTuple = {
// @deprecated Use localAddress.
localIp: '1.1.1.1',
localAddress: '1.1.1.1',
localPort: 1111,
remoteIp: '2.2.2.2',
remotePort: 2222,
Expand All @@ -554,7 +556,7 @@ test('WebRtcTransport events succeed', async () => {
const iceSelectedTupleChangeNotification =
new FbsWebRtcTransport.IceSelectedTupleChangeNotificationT(
new FbsTransport.TupleT(
iceSelectedTuple.localIp,
iceSelectedTuple.localAddress,
iceSelectedTuple.localPort,
iceSelectedTuple.remoteIp,
iceSelectedTuple.remotePort,
Expand Down
2 changes: 1 addition & 1 deletion node/src/test/test-node-sctp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ beforeEach(async () => {
ctx.udpSocket!.bind(0, '127.0.0.1', resolve);
});

const remoteUdpIp = ctx.plainTransport.tuple.localIp;
const remoteUdpIp = ctx.plainTransport.tuple.localAddress;
const remoteUdpPort = ctx.plainTransport.tuple.localPort;
const { OS, MIS } = ctx.plainTransport.sctpParameters!;

Expand Down
2 changes: 1 addition & 1 deletion rust/benches/producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ async fn init() -> (Worker, Router, WebRtcTransport, WebRtcTransport) {
WebRtcTransportOptions::new(WebRtcTransportListenInfos::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down
2 changes: 1 addition & 1 deletion rust/examples/echo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ impl EchoConnection {
WebRtcTransportOptions::new(WebRtcTransportListenInfos::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down
12 changes: 6 additions & 6 deletions rust/examples/multiopus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl EchoConnection {
let mut options = PlainTransportOptions::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down Expand Up @@ -198,9 +198,9 @@ impl EchoConnection {
RTCP listening on {}:{}\n \
PT=100\n \
SSRC=1111",
plain_transport.tuple().local_ip(),
plain_transport.tuple().local_address(),
plain_transport.tuple().local_port(),
plain_transport.rtcp_tuple().unwrap().local_ip(),
plain_transport.rtcp_tuple().unwrap().local_address(),
plain_transport.rtcp_tuple().unwrap().local_port(),
);

Expand All @@ -220,9 +220,9 @@ impl EchoConnection {
rtpbin.send_rtp_sink_0 \\\n \
rtpbin.send_rtp_src_0 ! udpsink host={} port={} sync=false async=false \\\n \
rtpbin.send_rtcp_src_0 ! udpsink host={} port={} sync=false async=false",
plain_transport.tuple().local_ip(),
plain_transport.tuple().local_address(),
plain_transport.tuple().local_port(),
plain_transport.rtcp_tuple().unwrap().local_ip(),
plain_transport.rtcp_tuple().unwrap().local_address(),
plain_transport.rtcp_tuple().unwrap().local_port(),
);

Expand All @@ -234,7 +234,7 @@ impl EchoConnection {
WebRtcTransportListenInfos::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down
2 changes: 1 addition & 1 deletion rust/examples/svc-simulcast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ impl SvcSimulcastConnection {
WebRtcTransportOptions::new(WebRtcTransportListenInfos::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down
2 changes: 1 addition & 1 deletion rust/examples/videoroom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ mod participant {
WebRtcTransportOptions::new(WebRtcTransportListenInfos::new(ListenInfo {
protocol: Protocol::Udp,
ip: IpAddr::V4(Ipv4Addr::LOCALHOST),
announced_ip: None,
announced_address: None,
port: None,
flags: None,
send_buffer_size: None,
Expand Down
44 changes: 27 additions & 17 deletions rust/src/data_structures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ impl AppData {
/// Listening protocol, IP and port for [`WebRtcServer`](crate::webrtc_server::WebRtcServer) to listen on.
///
/// # Notes on usage
/// If you use "0.0.0.0" or "::" as ip value, then you need to also provide `announced_ip`.
/// If you use "0.0.0.0" or "::" as ip value, then you need to also provide
/// `announced_address`.
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct ListenInfo {
Expand All @@ -61,7 +62,7 @@ pub struct ListenInfo {
/// Announced IPv4, IPv6 or hostname (useful when running mediasoup behind
/// NAT with private IP).
#[serde(skip_serializing_if = "Option::is_none")]
pub announced_ip: Option<String>,
pub announced_address: Option<String>,
/// Listening port.
#[serde(skip_serializing_if = "Option::is_none")]
pub port: Option<u16>,
Expand All @@ -84,7 +85,10 @@ impl ListenInfo {
Protocol::Udp => transport::Protocol::Udp,
},
ip: self.ip.to_string(),
announced_ip: self.announced_ip.clone().map(|ip| ip.to_string()),
announced_address: self
.announced_address
.clone()
.map(|address| address.to_string()),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Two allocations look strange. I'm pretty sure this will also work:

Suggested change
announced_address: self
.announced_address
.clone()
.map(|address| address.to_string()),
announced_address: self
.announced_address
.as_ref()
.map(|address| address.to_string()),

Though this is not new, it was like this before, I just comment on what I noticed.

Copy link
Member Author

Choose a reason for hiding this comment

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

You mean that this thing should also be done in other changes I did?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe, but I was commenting just about this one

Copy link
Member Author

Choose a reason for hiding this comment

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

On it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I don't see any other place in which as_ref() can be used instead of cloning.

port: self.port.unwrap_or(0),
flags: Box::new(self.flags.unwrap_or_default().to_fbs()),
send_buffer_size: self.send_buffer_size.unwrap_or(0),
Expand Down Expand Up @@ -231,7 +235,7 @@ pub struct IceCandidate {
/// The assigned priority of the candidate.
pub priority: u32,
/// The IP address or hostname of the candidate.
pub ip: String,
pub address: String,
/// The protocol of the candidate.
pub protocol: Protocol,
/// The port for the candidate.
Expand All @@ -248,7 +252,7 @@ impl IceCandidate {
Self {
foundation: candidate.foundation.clone(),
priority: candidate.priority,
ip: candidate.ip.clone(),
address: candidate.address.clone(),
protocol: Protocol::from_fbs(candidate.protocol),
port: candidate.port,
r#type: IceCandidateType::from_fbs(candidate.type_),
Expand Down Expand Up @@ -293,14 +297,14 @@ impl IceState {
/// `PipeTransport`, or via dynamic detection as it happens in `WebRtcTransport` (in which the
/// remote media address is detected by ICE means), or in `PlainTransport` (when using `comedia`
/// mode).
#[derive(Debug, Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Deserialize, Serialize)]
#[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Deserialize, Serialize)]
#[serde(untagged)]
pub enum TransportTuple {
/// Transport tuple with remote endpoint info.
#[serde(rename_all = "camelCase")]
WithRemote {
/// Local IP address.
local_ip: IpAddr,
/// Local IP address or hostname.
local_address: String,
/// Local port.
local_port: u16,
/// Remote IP address.
Expand All @@ -313,8 +317,8 @@ pub enum TransportTuple {
/// Transport tuple without remote endpoint info.
#[serde(rename_all = "camelCase")]
LocalOnly {
/// Local IP address.
local_ip: IpAddr,
/// Local IP address or hostname.
local_address: String,
/// Local port.
local_port: u16,
/// Protocol
Expand All @@ -323,10 +327,10 @@ pub enum TransportTuple {
}

impl TransportTuple {
/// Local IP address.
pub fn local_ip(&self) -> IpAddr {
let (Self::WithRemote { local_ip, .. } | Self::LocalOnly { local_ip, .. }) = self;
*local_ip
/// Local IP address or hostname.
pub fn local_address(&self) -> &String {
let (Self::WithRemote { local_address, .. } | Self::LocalOnly { local_address, .. }) = self;
local_address
}

/// Local port.
Expand Down Expand Up @@ -362,19 +366,25 @@ impl TransportTuple {
pub(crate) fn from_fbs(tuple: &transport::Tuple) -> TransportTuple {
match &tuple.remote_ip {
Some(_remote_ip) => TransportTuple::WithRemote {
local_ip: tuple.local_ip.parse().expect("Error parsing IP address"),
local_address: tuple
.local_address
.parse()
.expect("Error parsing local address"),
local_port: tuple.local_port,
remote_ip: tuple
.remote_ip
.as_ref()
.unwrap()
.parse()
.expect("Error parsing IP address"),
.expect("Error parsing remote IP address"),
remote_port: tuple.remote_port,
protocol: Protocol::from_fbs(tuple.protocol),
},
None => TransportTuple::LocalOnly {
local_ip: tuple.local_ip.parse().expect("Error parsing IP address"),
local_address: tuple
.local_address
.parse()
.expect("Error parsing local address"),
local_port: tuple.local_port,
protocol: Protocol::from_fbs(tuple.protocol),
},
Expand Down
Loading
Loading