-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
FLV: Refine source and http handler. v6.0.155 v7.0.14 #4165
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
winlinvip
force-pushed
the
bugfix/source-cleanup
branch
from
August 31, 2024 15:19
8fcfdd4
to
9d02e59
Compare
suzp1984
reviewed
Aug 31, 2024
winlinvip
force-pushed
the
bugfix/source-cleanup
branch
from
August 31, 2024 22:50
9d02e59
to
ba1f89a
Compare
This comment was marked as outdated.
This comment was marked as outdated.
suzp1984
approved these changes
Sep 1, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems good now.
winlinvip
changed the title
FLV: Refine source and http handler
FLV: Refine source and http handler. v6.0.155 v7.0.14
Sep 1, 2024
winlinvip
added a commit
that referenced
this pull request
Sep 1, 2024
1. Do not create a source when mounting FLV because it may not unmount FLV when freeing the source. If you access the FLV stream without any publisher, then wait for source cleanup and review the FLV stream again, there is an annoying warning message. ```bash HTTP #0 127.0.0.1:58026 GET http://localhost:8080/live/livestream.flv, content-length=-1 new live source, stream_url=/live/livestream http: mount flv stream for sid=/live/livestream, mount=/live/livestream.flv client disconnect peer. ret=1007 Live: cleanup die source, id=[], total=1 HTTP #0 127.0.0.1:58040 GET http://localhost:8080/live/livestream.flv, content-length=-1 serve error code=1097(NoSource)(No source found) : process request=0 : cors serve : serve http : no source for /live/livestream serve_http() [srs_app_http_stream.cpp:641] ``` > Note: There is an inconsistency. The first time, you can access the FLV stream and wait for the publisher, but the next time, you cannot. 2. Create a source when starting to serve the FLV client. We do not need to create the source when creating the HTTP handler. Instead, we should try to create the source in the cache or stream. Because the source cleanup does not unmount the HTTP handler, the handler remains after the source is destroyed. The next time you access the FLV stream, the source is not found. ```cpp srs_error_t SrsHttpStreamServer::hijack(ISrsHttpMessage* request, ISrsHttpHandler** ph) { SrsSharedPtr<SrsLiveSource> live_source; if ((err = _srs_sources->fetch_or_create(r.get(), server, live_source)) != srs_success) { } if ((err = http_mount(r.get())) != srs_success) { } srs_error_t SrsBufferCache::cycle() { SrsSharedPtr<SrsLiveSource> live_source = _srs_sources->fetch(req); if (!live_source.get()) { return srs_error_new(ERROR_NO_SOURCE, "no source for %s", req->get_stream_url().c_str()); } srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) { SrsSharedPtr<SrsLiveSource> live_source = _srs_sources->fetch(req); if (!live_source.get()) { return srs_error_new(ERROR_NO_SOURCE, "no source for %s", req->get_stream_url().c_str()); } ``` > Note: We should not create the source in hijack, instead, we create it in cache or stream: ```cpp srs_error_t SrsHttpStreamServer::hijack(ISrsHttpMessage* request, ISrsHttpHandler** ph) { if ((err = http_mount(r.get())) != srs_success) { } srs_error_t SrsBufferCache::cycle() { SrsSharedPtr<SrsLiveSource> live_source; if ((err = _srs_sources->fetch_or_create(req, server_, live_source)) != srs_success) { } srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) { SrsSharedPtr<SrsLiveSource> live_source; if ((err = _srs_sources->fetch_or_create(req, server_, live_source)) != srs_success) { } ``` > Note: This fixes the failure and annoying warning message, and maintains consistency by always waiting for the stream to be ready if there is no publisher. 3. Fail the http request if the HTTP handler is disposing, and also keep the handler entry when disposing the stream, because we should dispose the handler entry and stream at the same time. ```cpp srs_error_t SrsHttpStreamServer::http_mount(SrsRequest* r) { entry = streamHandlers[sid]; if (entry->disposing) { return srs_error_new(ERROR_STREAM_DISPOSING, "stream is disposing"); } void SrsHttpStreamServer::http_unmount(SrsRequest* r) { std::map<std::string, SrsLiveEntry*>::iterator it = streamHandlers.find(sid); SrsUniquePtr<SrsLiveEntry> entry(it->second); entry->disposing = true; ``` > Note: If the disposal process takes a long time, this will prevent unexpected behavior or access to the resource that is being disposed of. 4. In edge mode, the edge ingester will unpublish the source when the last consumer quits, which is actually triggered by the HTTP stream. While it also waits for the stream to quit when the HTTP unmounts, there is a self-destruction risk: the HTTP live stream object destroys itself. ```cpp srs_error_t SrsLiveStream::serve_http(ISrsHttpResponseWriter* w, ISrsHttpMessage* r) { SrsUniquePtr<SrsLiveConsumer> consumer(consumer_raw); // Trigger destroy. void SrsHttpStreamServer::http_unmount(SrsRequest* r) { for (;;) { if (!cache->alive() && !stream->alive()) { break; } // A circle reference. mux.unhandle(entry->mount, stream.get()); // Free the SrsLiveStream itself. ``` > Note: It also introduces a circular reference in the object relationships, the stream reference to itself when unmount: ```text SrsLiveStream::serve_http -> SrsLiveConsumer::~SrsLiveConsumer -> SrsEdgeIngester::stop -> SrsLiveSource::on_unpublish -> SrsHttpStreamServer::http_unmount -> SrsLiveStream::alive ``` > Note: We should use an asynchronous worker to perform the cleanup to avoid the stream destroying itself and to prevent self-referencing. ```cpp void SrsHttpStreamServer::http_unmount(SrsRequest* r) { entry->disposing = true; if ((err = async_->execute(new SrsHttpStreamDestroy(&mux, &streamHandlers, sid))) != srs_success) { } ``` > Note: This also ensures there are no circular references and no self-destruction. --------- Co-authored-by: Jacob Su <suzp1984@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Co-authored-by: Jacob Su suzp1984@gmail.com