ThibaultBee / StreamPack

SRT/RTMP/RTMPS live streaming libraries for Android
https://thibaultbee.github.io/StreamPack/index.html
Apache License 2.0
175 stars 67 forks source link

[Bug]: SrtProducer.connect(url) always throws "unknown host" when onConnectionListener is null #70

Closed geosir closed 1 year ago

geosir commented 1 year ago

Version

2.5.2

Environment that reproduces the issue

N/A

RTMP/SRT/... Server

N/A

Audio configuration

No response

Video configuration

No response

Is it reproducible in the demos application?

Not tested

Reproduction steps

Call srtProducer.connect(url) where url is a valid SRT URL.

Expected result

The method returns successfully.

Actual result

InvalidParameterException: Failed to parse URL <xx>: unknown host is thrown (from SrtProducer.kt:89)

Additional context

Hi! I encountered this small corner case while extending the SRT streamer and playing with it in a toy setup.

It appears that the safe call at SrtProducer.kt:120 will return null when onConnectionListener has not been set.

This seems to cause SrtProducer.kt:88 to evaluate to null even when uri.host is valid and the connection has succeeded. So it throws the error even when everything has worked.

I believe a possible fix might be to return true or Unit at the end of the connect(ip, port) method so that it's not null when successful. (Though I'm not sure of the code design/style considerations here, so I thought it better to just note the issue rather than submit a PR.)

Cheers, George

Relevant logs output

No response

ThibaultBee commented 1 year ago

Hi George,

Thanks for reporting this lovely issue and for the analysis 👍. Sorry about it. I have made a fix on main branch. Could you test it?

Best regards, Thibault

geosir commented 1 year ago

Hi Thibault, thanks, looks like it's fixed! Although I didn't get a chance to migrate from 2.5.2 to the changes in main, I was able to

  1. confirm that I could still replicate the error in my setup by not setting onConnectionListener, and
  2. confirm that, after replacing SrtProducer with a patched class copied exactly from the updated code in main, the error no longer occurs even with onConnectionListener still unset.

Nice fix too - super clean :)

Best, George