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

LiveSource: Refine fetch for external exposed interface. #2873

Merged

Conversation

chundonglinlin
Copy link
Member

@chundonglinlin chundonglinlin commented Jan 11, 2022

LiveSource's fetch function should be extended Parameters, because it update the request.

@chundonglinlin chundonglinlin changed the title LiveSource: Refine fetch_src for external exposed interface. LiveSource: Refine fetch for external exposed interface. Jan 11, 2022
@@ -451,7 +451,7 @@ class SrsLiveSourceManager : public ISrsHourGlass
public:
// Get the exists source, NULL when not exists.
// update the request and return the exists source.
virtual SrsLiveSource* fetch(SrsRequest* r);
virtual SrsLiveSource* fetch(SrsRequest* r, bool update = true);
Copy link
Member

@winlinvip winlinvip Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modification is the simplest, but it lacks maintainability and does not solve the fundamental problem.

Functions like fetch, which are similar to get, can never update.

Adding a parameter cannot solve this problem. Therefore, the fetch function should be separated into an API.

TRANS_BY_GPT3

Copy link
Member Author

@chundonglinlin chundonglinlin Jan 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the update operation from fetch and place update in the corresponding position where the request needs to be updated, such as in fetch_or_create requiring an update to the request.

TRANS_BY_GPT3

@winlinvip winlinvip self-assigned this Jan 12, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 12, 2022

Codecov Report

Merging #2873 (82bdfda) into 4.0release (c30aa47) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           4.0release    #2873   +/-   ##
===========================================
  Coverage       60.22%   60.22%           
===========================================
  Files             121      121           
  Lines           51070    51070           
===========================================
  Hits            30756    30756           
  Misses          20314    20314           

| Impacted Files | Coverage Δ | |'

Translated to English while maintaining the markdown structure:

'| Impacted Files | Coverage Δ | |
|---|---|---|
| trunk/src/app/srs_app_rtc_source.cpp | 12.39% <0.00%> (ø) | |
| trunk/src/app/srs_app_rtc_source.hpp | 9.09% <ø> (ø) | |
| trunk/src/app/srs_app_source.cpp | 0.66% <0.00%> (ø) | |

'

Translated to English while maintaining the markdown structure:

'| trunk/src/app/srs_app_rtc_source.cpp | 12.39% <0.00%> (ø) | |
| trunk/src/app/srs_app_rtc_source.hpp | 9.09% <ø> (ø) | |
| trunk/src/app/srs_app_source.cpp | 0.66% <0.00%> (ø) | |


Continue to review full report at Codecov.

Legend - Click here to learn more
| Δ = absolute <relative> (impact), ø = not affected, ? = missing data'

Translated to English while maintaining the markdown structure:

'| Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update c30aa47...82bdfda. Read the comment docs.

TRANS_BY_GPT3

@winlinvip
Copy link
Member

NICE 👍

@winlinvip winlinvip merged commit 7580341 into ossrs:4.0release Jan 13, 2022
@winlinvip winlinvip added the TransByAI Translated by AI/GPT. label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TransByAI Translated by AI/GPT.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants