Transcode: Bugfix: Fix loop transcoding with host. #3516. v6.0.168 v7.0.41#4325
Merged
winlinvip merged 7 commits intoossrs:developfrom Jun 4, 2025
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
trunk/src/app/srs_app_encoder.cpp:243
- Passing req->vhost twice may be unintentional. Please verify the parameter order for srs_generate_rtmp_url to ensure that all components of the URL are correct.
std::string input = srs_generate_rtmp_url(SRS_CONSTS_LOCALHOST, req->port, req->vhost, req->vhost, req->app, req->stream, req->param);
This was referenced Apr 15, 2025
Closed
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR addresses bug #3516 by correcting the RTMP URL formatting in the SrsEncoder::initialize_ffmpeg function.
- Replaces manual string concatenation with srs_generate_rtmp_url.
- Modifies parameter ordering to address the inversion of 'stream' and 'vhost'.
trunk/src/app/srs_app_encoder.cpp
Outdated
| input += "/"; | ||
| input += req->stream; | ||
|
|
||
| std::string input = srs_generate_rtmp_url(SRS_CONSTS_LOCALHOST, req->port, req->vhost, req->vhost, req->app, req->stream, req->param); |
There was a problem hiding this comment.
The function call to srs_generate_rtmp_url explicitly passes 'req->vhost' twice; please verify that the parameter order is correct to ensure the URL is generated with 'stream' and 'vhost' in the intended positions.
Suggested change
| std::string input = srs_generate_rtmp_url(SRS_CONSTS_LOCALHOST, req->port, req->vhost, req->vhost, req->app, req->stream, req->param); | |
| std::string input = srs_generate_rtmp_url(SRS_CONSTS_LOCALHOST, req->port, req->vhost, req->stream, req->app, req->stream, req->param); |
Contributor
There was a problem hiding this comment.
Is this patch really fix the bug?
The bug said the transcode stream can't output to same srs instance with same vhost.
output rtmp://127.0.0.1:[port]/[app]/[stream]_[engine]?vhost=[vhost];
It seems this PR didn't improve anything.
Member
Author
There was a problem hiding this comment.
The root cause of the issue lies in the order of the vhosts, which leads this segment of code to fail; therefore, merely rearranging the order resolves the problem.
Member
Author
There was a problem hiding this comment.
I've revised the code, which should render it more comprehensible.
deeb005 to
e464d9b
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR reorders the URL parameter placement in the encoder initialization to resolve bugs related to parameter mismatches.
- Reorders the placement of the "vhost" parameter from before the stream to after the stream.
- Aims to fix looping issues by aligning the input and output URL formats.
Comments suppressed due to low confidence (1)
trunk/src/app/srs_app_encoder.cpp:252
- The reordering of the URL parameters appears to address the reported issue, but please verify that the new sequence handles edge cases such as streams with existing query parameters; consider adding unit tests to ensure robustness.
input += "?vhost=";
xiaozhihong
approved these changes
Apr 17, 2025
suzp1984
approved these changes
Apr 17, 2025
winlinvip
approved these changes
Jun 4, 2025
winlinvip
added a commit
that referenced
this pull request
Jun 4, 2025
…4325) for issue: #3516 #4055 #3618 The issue arises from a mismatch between the `input` and `output` formats within the [`SrsEncoder::initialize_ffmpeg`](https://github.com/ossrs/srs/pull/4325/files#diff-a3dd7c498fc26d36def2e8c2c3b7edfe1bf78f0620b1a838aefa70ba119cad03L241-L254) function. For example: Input: `rtmp://127.0.0.1:1935/live?vhost=__defaultVhost__/livestream_ff` Output: `rtmp://127.0.0.1:1935/live/livestream_ff?vhost=__defaultVhost__` This may result in the failure of the [code segment](https://github.com/ossrs/srs/pull/4325/files#diff-a3dd7c498fc26d36def2e8c2c3b7edfe1bf78f0620b1a838aefa70ba119cad03L292-L298) responsible for determining whether to loop. It simply involves modifying the order of `stream` and `vhost`. The commit introducing this bug is: 7d47017 The order of [parameters in the configuration file](7d47017#diff-428de168925d659dae72bb49273c3b048ed2800906c6848560badae854250126L26-R26) has been modified to address the `ingest` issue. Please note that this PR does not entirely resolve the issue; for example, modifying the `output` format in configuration still results in exceptions. To comprehensively address this problem, extensive code modifications would be required. However, strictly adhering to the configuration file format can effectively prevent this issue. --------- Co-authored-by: Jacob Su <suzp1984@gmail.com> Co-authored-by: john <hondaxiao@tencent.com> Co-authored-by: winlin <winlinvip@gmail.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What issue has been resolved?
for issue: #3516 #4055 #3618
What is the root cause of the problem?
The issue arises from a mismatch between the
inputandoutputformats within theSrsEncoder::initialize_ffmpegfunction.For example:
Input:
rtmp://127.0.0.1:1935/live?vhost=__defaultVhost__/livestream_ffOutput:
rtmp://127.0.0.1:1935/live/livestream_ff?vhost=__defaultVhost__This may result in the failure of the code segment responsible for determining whether to loop.
What is the approach to solving this issue?
It simply involves modifying the order of
streamandvhost.How was the issue introduced?
The commit introducing this bug is: 7d47017
The order of parameters in the configuration file has been modified to address the
ingestissue.Outstanding issues
Please note that this PR does not entirely resolve the issue; for example, modifying the
outputformat in configuration still results in exceptions. To comprehensively address this problem, extensive code modifications would be required.However, strictly adhering to the configuration file format can effectively prevent this issue.
Co-authored-by: Jacob Su suzp1984@gmail.com
Co-authored-by: john hondaxiao@tencent.com