versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.18k stars 1.12k forks source link

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

Closed ibc closed 7 months ago

ibc commented 8 months ago

Fixes #1323

NOTE: Why has been TransportTuple.localIp renamed to TransportTuple.localAddress? Because indeed its value is an announced address if a hostname was given in announcedAddress in a TransportListenInfo.

ibc commented 8 months ago

TODO: Once merged/released, docs in website must be updated.

ibc commented 8 months ago

Missing thing, local_address in TransportTuple in Rust must not be an IPAddr but a String. On it.

ibc commented 7 months ago

I need help to resolve this:

error[E0507]: cannot move out of dereference of `parking_lot::lock_api::MutexGuard<'_, parking_lot::RawMutex, TransportTuple>`
   --> rust/src/router/pipe_transport.rs:727:9
    |
727 |         *self.inner.data.tuple.lock()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move occurs because value has type `TransportTuple`, which does not implement the `Copy` trait

error[E0382]: borrow of moved value: `tuple`
   --> rust/src/router/plain_transport.rs:691:56
    |
688 |                         Notification::Tuple { tuple } => {
    |                                               ----- move occurs because `tuple` has type `TransportTuple`, which does not implement the `Copy` trait
689 |                             *data.tuple.lock() = tuple;
    |                                                  ----- value moved here
690 |
691 |                             handlers.tuple.call_simple(&tuple);
    |                                                        ^^^^^^ value borrowed here after move

In the first error I've tried *self.inner.data.tuple.clone().lock() but:

error[E0599]: no method named `clone` found for struct `parking_lot::lock_api::Mutex` in the current scope
   --> rust/src/router/pipe_transport.rs:727:32
    |
727 |         *self.inner.data.tuple.clone().lock()
    |                                ^^^^^ method not found in `Mutex<RawMutex, TransportTuple>`

This mutex/lock thing is above my (increasing) little knowledge, so @nazar-pc help plz.

nazar-pc commented 7 months ago

I think you want to clone inner data, in which case it should be lock().clone()

ibc commented 7 months ago

I think you want to clone inner data, in which case it should be lock().clone()

I tried it already but...

error[E0614]: type `TransportTuple` cannot be dereferenced
   --> rust/src/router/pipe_transport.rs:727:9
    |
727 |         *self.inner.data.tuple.lock().clone()
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
nazar-pc commented 7 months ago

.lock() method returns something like MutexGruard. This is a wrapper type that locks unlocks Mutex when dropped. This MutexGuard can be dereferenced to the type mutex contains. So *mutex.lock() is essentially similar to this:

let number = 5;
let number_copy = *&number;

Since the type you have is no longer Copy, you need to clone it explicitly.

So what was *self.inner.data.tuple.lock() before (can be also written as self.inner.data.tuple.lock().clone() explicitly) now needs to be written as self.inner.data.tuple.lock().clone() due to explicit cloning.

So essentially you'll do this:

let s = "Hello".to_string();
let s_copy = (&s).clone();
ibc commented 7 months ago

So indeed replacing *self.inner.data.tuple.lock() with self.inner.data.tuple.lock().clone() works although I cannot understand how both things can return same type. AFAIU xxxx and xxxx.clone() return the same type, so how can *xxxx and xxxx.clone() return the same type as well?

ibc commented 7 months ago

In addition I've also fixed another issue as follows:

error[E0382]: borrow of moved value: `tuple`
   --> rust/src/router/plain_transport.rs:691:56
    |
688 |                         Notification::Tuple { tuple } => {
    |                                               ----- move occurs because `tuple` has type `TransportTuple`, which does not implement the `Copy` trait
689 |                             *data.tuple.lock() = tuple;
    |                                                  ----- value moved here
690 |
691 |                             handlers.tuple.call_simple(&tuple);
    |                                                        ^^^^^^ value borrowed here after move

Solution in line 689:

*data.tuple.lock() = tuple.clone();

Hope it's ok. IMHO it is since it follows same rationale as discussed in above comments.

nazar-pc commented 7 months ago

MutexGuard doesn't implement Clone, so compiler will try to call Clone on the type that it dereferences to, which is inner value you actually want. https://doc.rust-lang.org/book/ch15-02-deref.html

ibc commented 7 months ago

Ok this is ready for review. @nazar-pc please.

ibc commented 7 months ago

Moving back to draft PR because I forgot lot of things.

ibc commented 7 months ago

@nazar-pc, I should also update Changelog and versions in Rust, right?

nazar-pc commented 7 months ago

@nazar-pc, I should also update Changelog and versions in Rust, right?

Yes, please

ibc commented 7 months ago

Rust versions bump done.