meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.11k stars 2.46k forks source link

RTSP SETUP for URL with parameters fails #1869

Closed outofrange closed 4 years ago

outofrange commented 4 years ago

I'm currently testing #1866, but I'm quite positive this bug is happening on master as well (I could not find changes to relevant code parts when looking for the problem); if there is any doubt, I'm happy to actually test it with the latest release as well.

When configuring a RTSP streaming mountpoint URL

url = "rtsp://192.168.2.10/axis-media/media.amp?videopsenabled=1"

, a RTSP DESCRIBE is responded with the correct (video) control URL rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1.

Unfortunately, when using URL parameters, Janus fails to check that The control attribute already contains the whole URL:

if(strstr(vcontrol, source->rtsp_url) == vcontrol) {
    /* The control attribute already contains the whole URL? */
    g_snprintf(uri, sizeof(uri), "%s", vcontrol);
} else {
    /* Append the control attribute to the URL */
    g_snprintf(uri, sizeof(uri), "%s/%s", source->rtsp_url, vcontrol);
}

Since Janus doesn't ignore URL parameters when looking for rtsp_url in vcontrol, the wrong branch will be executed, sending this request:

SETUP rtsp://192.168.2.10/axis-media/media.amp?videopsenabled=1/rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1 RTSP/1.0

which is answered with a 404.

I (or the camera, I guess) would expect a request like

SETUP rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1 RTSP/1.0

lminiero commented 4 years ago

Thanks for spotting this. I'll be out all week for the IETF, so can't look at it now. In case you can come up with a patch to submit as a pull request in the meanwhile, though, I'll gladly review :+1:

outofrange commented 4 years ago

I don't know how to write C++, so I don't think that would be a good idea :D

I'll try to workaround the issue for now; thanks for your quick response!

lminiero commented 4 years ago

@outofrange can you test the patch above?

outofrange commented 4 years ago

The behaviour has somewhat changed*, but SETUP is still incorrect:

SETUP rtsp://192.168.2.10/axis-media/media.amp/stream=0?videopsenabled=1?videopsenabled=1 RTSP/1.0

as well as

SETUP rtsp://192.168.2.10/axis-media/media.amp?videopsenabled=1/rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1?videopsenabled=1 RTSP/1.0

I've created a Gist for Janus' streaming debug log (including SDP, a=control and I think 2 re-establishments of the RTSP stream): https://gist.github.com/outofrange/06da14751bfd16c118c5cbf26abdc568 (Unfortunately, Logstash and UDP might've messed up the exact ordering of the log; if there are any ambiguities, I'll get another log directly from Janus stdout/logfile)

(* now, the stream can be established and video can be streamed, but the connection is reset once a minute or so, and there is no audio)

lminiero commented 4 years ago

@outofrange any chance you can share a .pcap dump of the RTSP exchange?

lminiero commented 4 years ago

Neverming that, I can see that these are the control attributes for video and audio respectively:

a=control:rtsp://192.168.2.10/axis-media/media.amp/stream=0?videopsenabled=1
a=control:rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1

but they get translated to these urls:

[video] rtsp://192.168.2.10/axis-media/media.amp/stream=0?videopsenabled=1?videopsenabled=1
[audio] rtsp://192.168.2.10/axis-media/media.amp?videopsenabled=1/rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1?videopsenabled=1

So for video we're simply adding the query string part twice, which is probably harmless and why video works; for audio, instead, there's something wrong as the whole url is passed again and becomes part of the query string, which I guess confuses the hell out of the camera (and rightly so). Not sure why this is only happening for audio and not for video: maybe because it's the second stream, and there's some leftover buffer or copy/paste error. I'll try to come up with a patch to fix this.

Since I'm operating blind, though, if you have a way to expose that camera for some testing that would help.

lminiero commented 4 years ago

@outofrange should be fixed in https://github.com/meetecho/janus-gateway/pull/1875/commits/aa6797c5172e4508ba169d67e9bb2b1d8c48545a, please let me know if that's not the case.

lminiero commented 4 years ago

@outofrange ping :slightly_smiling_face:

outofrange commented 4 years ago

Looks good, video and audio is working:

SETUP rtsp://192.168.2.10/axis-media/media.amp/stream=0?videopsenabled=1 RTSP/1.0

SETUP rtsp://192.168.2.10/axis-media/media.amp/stream=1?videopsenabled=1 RTSP/1.0

Thank you!

outofrange commented 4 years ago

but the connection is reset once a minute or so

This still happens, but doesn't seem to be related to this issue. I will investigate further and open another issue when I know what's actually causing it.

lminiero commented 4 years ago

Are the URLs we craft now correct, though? If so, I can at least proceed and merge those fixes. We can worry about the other problem as soon as you open another issue (unless they are in fact related).

outofrange commented 4 years ago

Yes, that's what I meant :)

lminiero commented 4 years ago

Gotcha :+1: I'll merge the fix and close this issue, then. Looking forward to the other problem (not really but you get the point :smile: )