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

How to configure webhooks? #31

Closed
xoraingroup opened this issue Oct 22, 2020 · 57 comments
Closed

How to configure webhooks? #31

xoraingroup opened this issue Oct 22, 2020 · 57 comments

Comments

@xoraingroup
Copy link

Hi,

How do we configure webhooks like on_publish on_play etc?

Thank You

@q191201771
Copy link
Owner

Not support yet, this feature will be add in two weeks.

So far, lal only support HTTP API to query some server info, e.g. exist stream info.
check this out:

Thank you.

@xoraingroup
Copy link
Author

Great, the API is good, but the webhooks part is important for our use case. So do you think it will be done in two weeks?

@xoraingroup
Copy link
Author

We want to test thoroughly your project. We are very much interested in it.

@q191201771
Copy link
Owner

Great, the API is good, but the webhooks part is important for our use case. So do you think it will be done in two weeks?

Yeah, I suppose so, about next weekend.

We want to test thoroughly your project. We are very much interested in it.

All test and feedback are welcome.

@xoraingroup
Copy link
Author

Awesome , looking forward to see this.

@xoraingroup
Copy link
Author

Also please check , when i am sending some parameters in rtmp url, lal server strips those parameters. For example

rtmp://myawesome-lal-rtmp-server:1935/app/hello?token=asdfasdf

When I see logs, it removes token=asdfasdf from rtmp server. This is also very much needed and it should not be stripped or removed.

@q191201771
Copy link
Owner

Also please check , when i am sending some parameters in rtmp url, lal server strips those parameters. For example

rtmp://myawesome-lal-rtmp-server:1935/app/hello?token=asdfasdf

When I see logs, it removes token=asdfasdf from rtmp server. This is also very much needed and it should not be stripped or removed.

Yeah, I strip parameters.
see https://github.com/q191201771/lal/blob/master/pkg/rtmp/server_session.go#L325
Give me more info about how are you using those parameters.
Just log it? or webhooks?
Are you using lal as an server or libaray?
How about set those parameters in rtmp.ServerSession, and you can get it outside?

@xoraingroup
Copy link
Author

For webhooks and also in webhooks for push relay and pull relay . I am using lal as server.

@q191201771
Copy link
Owner

For webhooks and also in webhooks for push relay and pull relay . I am using lal as server.

Ok, I got it.

@xoraingroup
Copy link
Author

How does it look like? Have you started implementing it. I am eagerly waiting to test it.

@q191201771
Copy link
Owner

I'll do it on this weekend.

And I just noticed that you wanna relay push & pull.

lalserver aready support static const relay push & pull by conf file relay_push and relay_pull, you can check these out see if satisfy.

And also, I'll add some interface both in HTTP API and Webhook to support dynamic relay pull.

Maybe you can tell me more info about your scenario.

@xoraingroup
Copy link
Author

xoraingroup commented Oct 29, 2020

I'll do it on this weekend.

And I just noticed that you wanna relay push & pull.

lalserver aready support static const relay push & pull by conf file relay_push and relay_pull, you can check these out see if satisfy.

And also, I'll add some interface both in HTTP API and Webhook to support dynamic relay pull.

Maybe you can tell me more info about your scenario.

Yes I am testing that already - but using dynamic relay pull would be amazing .... Also don't strip parameters from strip key when using relay push and pull using config file.

Waiting for your updates.

I have another idea related to pull clustering via api, but i will discuss with you after wards. Its very interesting and doable.

@q191201771
Copy link
Owner

I've pushed some code to branch master. include:

  1. HTTP Notify(webhooks). doc: https://pengrl.com/p/20101
  2. Add a new HTTP API /api/ctrl/start_pull. doc: https://pengrl.com/p/20100

And, I've written a demo dispatch, which can manager multi lal node as a cluster by dynamic relay pull from each other.
(this demo do not 100% finished yet)

  1. Do not strip parameters by static const relay push

@xoraingroup
Copy link
Author

xoraingroup commented Oct 31, 2020

Amazing work. Just a quick note, can you add lal server address as well in all webhooks? Like hostname or ip?

About "dispatch" I will try and test all those.

@xoraingroup
Copy link
Author

xoraingroup commented Oct 31, 2020

I believe when you stop publishing the hook should be "on_pub_stop" but what it triggers is "on_sub_stop". Its a bug.

The issue is in config file.

on_pub_stop": "http://127.0.0.1:10101/on_sub_stop

  "http_notify": {
    "enable": true,
    "update_interval_sec": 5,
    "on_update": "http://127.0.0.1:10101/on_update",
    "on_pub_start": "http://127.0.0.1:10101/on_pub_start",
    "on_pub_stop": "http://127.0.0.1:10101/on_sub_stop",
    "on_sub_start": "http://127.0.0.1:10101/on_sub_start",
    "on_sub_stop": "http://127.0.0.1:10101/on_sub_stop"
  },

image

@xoraingroup
Copy link
Author

xoraingroup commented Oct 31, 2020

I've pushed some code to branch master. include:

  1. HTTP Notify(webhooks). doc: https://pengrl.com/p/20101
  2. Add a new HTTP API /api/ctrl/start_pull. doc: https://pengrl.com/p/20100

dynamic relay pull is not working. It should do 302 rtmp redirect to the server which has the stream. But it does not redirect and stream is not playable from other server which does not have stream.

Two rtmp servers :
localhost:19350
api port: 8083

localhost:19550
api: 8283

ffmpeg -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok
ffplay rtmp://localhost:19550/live/ok

image

And, I've written a demo dispatch, which can manager multi lal node as a cluster by dynamic relay pull from each other.
(this demo do not 100% finished yet)

  1. Do not strip parameters by static const relay push

@q191201771
Copy link
Owner

q191201771 commented Nov 1, 2020

Amazing work. Just a quick note, can you add lal server address as well in all webhooks? Like hostname or IP?

...

You can config each lal node with a different server_id in the config file, all webhooks will carry it.

@q191201771
Copy link
Owner

q191201771 commented Nov 1, 2020

~I believe when you stop publishing the hook should be "on_pub_stop" but what it triggers is "on_sub_stop". Its a bug. ~

The issue is in config file.

on_pub_stop": "http://127.0.0.1:10101/on_sub_stop

...

Ok, a config mistake, I've fixed it.

@q191201771
Copy link
Owner

q191201771 commented Nov 1, 2020

dynamic relay pull is not working. It should do 302 rtmp redirect to the server which has the stream. But it does not redirect and stream is not playable from other server which does not have stream.

...

LAL doesn't use 302 redirect to implement cluster.

Instead, the lal node which the client plays from will dynamically pull stream from the node which has the published stream.

Compare to 302 redirect, the benefits are:

  1. client doesn't need to support 302 redirect
  2. one TCP/RTMP connect will be enough, doesn't need to reconnect

Try:

$./bin/lalserver -c conf/lalserver.conf.json

$./bin/lalserver -c conf/node2.conf.json

$./bin/dispatch

$ffmpeg -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok

$ffplay rtmp://localhost:19550/live/ok

@xoraingroup
Copy link
Author

Amazing work. Just a quick note, can you add lal server address as well in all webhooks? Like hostname or IP?
...

You can config each lal node with a different server_id in the config file, all webhooks will carry it.

Yes thats possible, but I need at least following more parameters for my use case,

tcURL, Server IP and published stream id.

I need it for querying the stream id and possibly kicking out the stream at some point in future. This is very important for me. please 👍

@q191201771
Copy link
Owner

Yes thats possible, but I need at least following more parameters for my use case,

tcURL, Server IP and published stream id.

server_id is the same as hostname or ip by my view, you can mapping the info by yourself.

I need it for querying the stream id and possibly kicking out the stream at some point in future. This is very important for me. please 👍

Kicking out the stream is another feature, maybe I will add it to HTTP API next week.

@xoraingroup
Copy link
Author

xoraingroup commented Nov 1, 2020

dynamic relay pull is not working. It should do 302 rtmp redirect to the server which has the stream. But it does not redirect and stream is not playable from other server which does not have stream.
...

LAL doesn't use 302 redirect to implement cluster.

Instead, the lal node which the client plays from will dynamically pull stream from the node which has the published stream.

Compare to 302 redirect, the benefits are:

  1. client doesn't need to support 302 redirect
  2. one TCP/RTMP connect will be enough, doesn't need to reconnect

Try:

$./bin/lalserver -c conf/lalserver.conf.json

$./bin/lalserver -c conf/node2.conf.json

$./bin/dispatch

$ffmpeg -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok

$ffplay rtmp://localhost:19550/live/ok

Ok this works perfectly. However two issues i found.

  1. Once you start pushing using ffmpeg -re -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok and then stop it, the on_update call still carries the old data. It should be empty. Also on_update timer should be configurable or it should trigger only if there is some stream published.
  2. Try the following scenario.
$./bin/lalserver -c conf/lalserver.conf.json

$./bin/lalserver -c conf/node2.conf.json

$./bin/dispatch

$ffmpeg -re -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok

$ffplay rtmp://localhost:19550/live/ok

Then stop all publishing and pulling. Now instead of publishing same stream key to rtmp://localhost:19350/live/ok , publish it to rtmp://localhost:19550/live/ok . The following error is seen , even though there is no existing stream in servers.

image

@q191201771
Copy link
Owner

  1. Once you start pushing using ffmpeg -re -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok and then stop it, the on_update call still carries the old data. It should be empty.

I didn't see this. more details.

Also on_update timer should be configurable or it should trigger only if there is some stream published.

Fixed duration timer can not only use for report stream info, but also keep alive bwetten node and dispatch.

@q191201771
Copy link
Owner

Then stop all publishing and pulling. Now instead of publishing same stream key to rtmp://localhost:19350/live/ok , publish it to rtmp://localhost:19550/live/ok . The following error is seen , even though there is no existing stream in servers.
...

The reason is that pull session didn't release.

Two common methods to do it:

  1. release it by receiving audio/video data timeout
  2. release it while all sub session is empty

I'll do it next week.

@xoraingroup
Copy link
Author

  1. Once you start pushing using ffmpeg -re -i somevideo.mp4 -c copy -f flv rtmp://localhost:19350/live/ok and then stop it, the on_update call still carries the old data. It should be empty.

I didn't see this. more details.

Also on_update timer should be configurable or it should trigger only if there is some stream published.

Fixed duration timer can not only use for report stream info, but also keep alive bwetten node and dispatch.

Ok got it.

@xoraingroup
Copy link
Author

Then stop all publishing and pulling. Now instead of publishing same stream key to rtmp://localhost:19350/live/ok , publish it to rtmp://localhost:19550/live/ok . The following error is seen , even though there is no existing stream in servers.
...

The reason is that pull session didn't release.

Two common methods to do it:

  1. release it by receiving audio/video data timeout
  2. release it while all sub session is empty

I'll do it next week.

Yes that is important, otherwise in OBS usually if you disconnect and then try to reconnect, it will never be able to reconnect due to this issue.

@q191201771
Copy link
Owner

It should be like a timer that can be used when the stream finishes, it can do cleanup. For example, cleanup_after=30, So when the stream pushing is stopped, then after 30 second of that the hls cleanup should kick in to clean out hls fragments of that stream.

For different people, the cleanup timing is different, and it can even not decided by the stream server.
For example, someone wanna using HLS for the record, and do clean up after they transport these files to other storage systems.

You can clean up by yourself, and HTTP Notify on_pub_stop may be useful.
Or just clean up the expired file by a crontab task, e.g. 3 days ago file.

I'll add an HTTP API to cleanup the HLS file of the specific stream, as well.

In nginx and srs its on_connect, so I hope it would be the same.

For nginx is rtmp connect, I can add it to HTTP Notify.

@xoraingroup
Copy link
Author

It should be like a timer that can be used when the stream finishes, it can do cleanup. For example, cleanup_after=30, So when the stream pushing is stopped, then after 30 second of that the hls cleanup should kick in to clean out hls fragments of that stream.

Yes thats correct, but there should be an option in config as well which can be enabled or disabled and with some timer, so that the server itself can remove it. Just an opinion as its already done by almost all RTMP servers.

For different people, the cleanup timing is different, and it can even not decided by the stream server.
For example, someone wanna using HLS for the record, and do clean up after they transport these files to other storage systems.

You can clean up by yourself, and HTTP Notify on_pub_stop may be useful.
Or just clean up the expired file by a crontab task, e.g. 3 days ago file.

I'll add an HTTP API to cleanup the HLS file of the specific stream, as well.

In nginx and srs its on_connect, so I hope it would be the same.

For nginx is rtmp connect, I can add it to HTTP Notify.

Great and awesome. 👍

@xoraingroup
Copy link
Author

Did you already added on_connect web hook?

@q191201771
Copy link
Owner

Did you already added on_connect web hook?

I'll do it tomorrow.

@q191201771
Copy link
Owner

q191201771 commented Nov 7, 2020

I've pushed some code to branch master:

  1. Stop pull from another node while no one watching (in other words, stop pull if no sub)
  2. For the in-data type session, no data received for 10 seconds, lalserver will force close the session. and for the out-data type session, is no data wrote.

These two features will solve the cluster republish/replay issue, try again.

And, I've added on_rtmp_connect event to HTTP Notify.

Todo list:

  • hls clean up
  • kick out the specified session by HTTP API
  • demo/dispatch

@xoraingroup
Copy link
Author

Amazing work... Let me test this. Kudos to you for this effort. :)

@xoraingroup
Copy link
Author

xoraingroup commented Nov 7, 2020

Seems like in push relay you mistyped variable. It should be urlParam instead of url when passing parameters.

@xoraingroup
Copy link
Author

I've pushed some code to branch master:

  1. Stop pull from another node while no one watching (in other words, stop pull if no sub)
  2. For the in-data type session, no data received for 10 seconds, lalserver will force close the session. and for the out-data type session, is no data wrote.

These two features will solve the cluster republish/replay issue, try again.

Tested, working OK.

And, I've added on_rtmp_connect event to HTTP Notify.

Todo list:

  • hls clean up
  • kick out the specified session by HTTP API
  • demo/dispatch

These are the major stuff I am waiting to test. Do you think you can finish it in this weekend? I am doing lots of stress testing and different use cases.

@q191201771
Copy link
Owner

Seems like in push relay you mistyped variable. It should be urlParam instead of url when passing parameters.

Fixed.

@xoraingroup
Copy link
Author

Seems like in push relay you mistyped variable. It should be urlParam instead of url when passing parameters.

Fixed.

What about

hls clean up
kick out the specified session by HTTP API
demo/dispatch

Do you think you can do it today? I need to test few cases.

@q191201771
Copy link
Owner

No, I can't do it today. I'd think about it, and maybe do it next weekend, but don't take it as a promise.

@xoraingroup
Copy link
Author

Ok. no issues. Will be waiting for you update. By the way you haven't pushed the above urlParam change to master. Have you?

@q191201771
Copy link
Owner

By the way you haven't pushed the above urlParam change to master. Have you?

My mistake, try again.

@xoraingroup
Copy link
Author

I've pushed some code to branch master:

  1. Stop pull from another node while no one watching (in other words, stop pull if no sub)
  2. For the in-data type session, no data received for 10 seconds, lalserver will force close the session. and for the out-data type session, is no data wrote.

These two features will solve the cluster republish/replay issue, try again.

Tested, working OK.

And, I've added on_rtmp_connect event to HTTP Notify.
Todo list:

  • hls clean up
  • kick out the specified session by HTTP API
  • demo/dispatch

If you can do this quickly, then i suppose, i should close this ticket and create new ones. Otherwise there are alot of features and new implementation in this ticket. What do you say?

These are the major stuff I am waiting to test. Do you think you can finish it in this weekend? I am doing lots of stress testing and different use cases.

@xoraingroup
Copy link
Author

These webhooks are not useful. Because if the backend denies the request on webhook, then publishing, playing should not happen. But it allows this.

@q191201771
Copy link
Owner

These webhooks are not useful. Because if the backend denies the request on webhook, then publishing, playing should not happen. But it allows this.

Yeah, that's the design.
Lal doesn't provide sync auth mode, since it will block the client until lal receive a response from HTTP Notify. This is bad for all normal users.

kick out the specified session by HTTP API
When this job is done, you can do auth async.

@xoraingroup
Copy link
Author

xoraingroup commented Nov 11, 2020

How do the other media servers provide without blocking it for other users. I believe every rtmp connection is spawn in a new goroutine and for example on rtmp_connect or on_publish it sends webhook and waits for result - If the result is ok, it just continues otherwise it stops the user at that point. Its just I have seen it in all rtmp servers. In above design the only useful webhook is on_update. What is your take on that?

@q191201771
Copy link
Owner

I'm not saying that the authing client would block other clients.
I'm saying that it's bad to let every client waiting for its auth response synchronously, even though in most scenarios they are the normal client

You can do auth stuff in webhooks, and call kick out HTTP API if auth failed.

Every design has its focus. Lal prefers the user experience of normal scenarios and semantic clarity.

@xoraingroup
Copy link
Author

Yes thats a good point, but it creates unnecessary traffic. If the user can be denied in the initial phase, its much better than to tell server to kick out session. But your point is also valid.

What about hls cleanup API. I am getting thousands of files while testing stream. Can you provide some sort of minimal API quickly? Thank you

@q191201771
Copy link
Owner

HLS cleanup:

You can config HLS cleanup flag(cleanup_flag) in the conf file.

If you enable this, when pub session stop, lal will delete relative HLS file after <fragment_duration_ms> * <fragment_num> * 2

kick out session:

You can kick out the session by HTTP API /api/ctrl/kick_out_session

HTTP API doc: https://pengrl.com/p/20100/

@xoraingroup
Copy link
Author

Tested and working perfectly good. Both kickout session and hls cleanup. Well done for awesome work.

I would like to ask again, if its possible for you to wait for webhook response and then allow stream based on backend response. This is the only and only thing missing in this product for me at least. May be in a separate branch. Please :)

@q191201771
Copy link
Owner

Before the backend response, you wanna lal keep sending audio/video data to the client or not?

@xoraingroup
Copy link
Author

No, audio video till there is a response. If the status code is 200 with text as '0', then allow the stream otherwise don't.

@xoraingroup
Copy link
Author

Any updates on this?

@q191201771
Copy link
Owner

No, I have no time doing sync auth. Move to indefinite delay list. #37
If you have other issues, open another one.

@xoraingroup
Copy link
Author

Ok thank you for time till now. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants