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.

Sign up for GitHub

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

Fix bug for HTTP, failed to download large file recorded with 2GB #2780 #2781

Closed
wants to merge 1 commit into from
Closed

Fix bug for HTTP, failed to download large file recorded with 2GB #2780 #2781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions trunk/src/app/srs_app_http_static.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ srs_error_t SrsVodStream::serve_flv_stream(ISrsHttpResponseWriter* w, ISrsHttpMe
return err;
}

srs_error_t SrsVodStream::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, string fullpath, int start, int end)
srs_error_t SrsVodStream::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, string fullpath, int start, long end)
Copy link
Member

@chundonglinlin chundonglinlin Dec 22, 2021 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

It is more appropriate to use int64_t instead of long.

TRANS_BY_GPT3

{
srs_error_t err = srs_success;

Expand All @@ -154,7 +154,7 @@ srs_error_t SrsVodStream::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMe

// parse -1 to whole file.
if (end == -1) {
end = (int)(fs->filesize() - 1);
end = fs->filesize() - 1;
}

if (end > fs->filesize() || start > end || end < 0) {
Expand All @@ -180,8 +180,8 @@ srs_error_t SrsVodStream::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMe
fs->seek2(start);

// send data
if ((err = copy(w, fs, r, (int)left)) != srs_success) {
return srs_error_wrap(err, "read mp4=%s size=%d", fullpath.c_str(), (int)left);
if ((err = copy(w, fs, r, left)) != srs_success) {
Copy link
Member

@chundonglinlin chundonglinlin Dec 22, 2021 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

If only the left variable type is changed to int64_t, but the declaration of copy(ISrsHttpResponseWriter* w, SrsFileReader* fs, ISrsHttpMessage* r, int64_t size) should also be modified to int64_t, and the variable types inside the copy function should remain consistent, otherwise the modification will not be achieved.

TRANS_BY_GPT3

Copy link
Member

@winlinvip winlinvip Dec 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Could you please create a new PR? @CodeflowSun mentioned that he works with Java, so it may not be suitable for him to continue modifying the C++ code. Please make sure to maintain the markdown structure.

@CodeflowSun has already identified the problem, which is already very good. Please make sure to maintain the markdown structure.

Thank you very much to both of you. 🙏 Please make sure to maintain the markdown structure.

TRANS_BY_GPT3

Copy link
Member

@chundonglinlin chundonglinlin Dec 23, 2021 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

Okay, I will create a new PR to address this issue. Thanks to @CodeflowSun for helping to identify the problem 🙏.

TRANS_BY_GPT3

Copy link
Member

@chundonglinlin chundonglinlin Dec 23, 2021 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

When modifying the distribution of mp4 files, it was discovered that flv files also have this issue. offset is defined as int type. If you use ffplay http://127.0.0.1:8080/test.flv, the flv file will keep buffering after exceeding 2GB. After making modifications, it was found that flv does not support range operations. Summarizing the above observations:

  1. When distributing mp4 and flv files, if they exceed 2GB, there will be overflow or connection timeout issues.
  2. When distributing flv files, VLC player will exit or fail when dragging.

TRANS_BY_GPT3

Copy link
Member

@chundonglinlin chundonglinlin Dec 23, 2021 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

When modifying the distribution of mp4 files, it was found that flv also had this problem. offset is defined as int type. If ffplay http://127.0.0.1:8080/test.flv is used, the flv file will freeze indefinitely after exceeding 2GB. After modification, it was found that range operation is not supported for flv. Summarize the above phenomena: 1. When distributing mp4 and flv files, if they exceed 2GB, overflow or connection timeout may occur. 2. When distributing flv files, VLC will exit or fail when dragging during playback.

Phenomenon 2, when dragging to play flv, does not support the range parameter. Is it necessary? When distributing flv files using nginx, it supports range operation.

TRANS_BY_GPT3

Copy link

@ellenWei11 ellenWei11 Aug 5, 2022 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

Hi, I would like to ask if the drag-and-drop feature for FLV files has been resolved?

TRANS_BY_GPT3

Copy link

@ellenWei11 ellenWei11 Aug 5, 2022 •
edited by winlinvip
Loading

Choose a reason for hiding this comment

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

I compiled nginx with the following configure arguments: --prefix=/usr/local/nginx --with-http_stub_status_module --with-http_ssl_module --with-http_gzip_static_module --with-http_mp4_module --with-http_flv_module.
At the same time, I used yamdi to add keyframe information to the recorded videos.
yamdi -i source.flv -o dest.flv //input video output video
Then, I can use nginx to play the recorded files and enable dragging and dropping.
So, the problem should be that the DVR recording does not include keyframe information, which is why dragging and dropping playback is not possible.

TRANS_BY_GPT3

return srs_error_wrap(err, "read mp4=%s size=%" PRId64 , fullpath.c_str(), left);
}

return err;
Expand Down
10 changes: 5 additions & 5 deletions trunk/src/app/srs_app_http_static.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@

#include <srs_app_http_conn.hpp>

struct SrsM3u8CtxInfo
{
srs_utime_t request_time;
SrsRequest* req;
struct SrsM3u8CtxInfo
{
srs_utime_t request_time;
SrsRequest* req;
};

// The flv vod stream supports flv?start=offset-bytes.
Expand All @@ -31,7 +31,7 @@ class SrsVodStream : public SrsHttpFileServer, public ISrsFastTimer
virtual ~SrsVodStream();
protected:
virtual srs_error_t serve_flv_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath, int offset);
virtual srs_error_t serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath, int start, int end);
virtual srs_error_t serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath, int start, long end);
virtual srs_error_t serve_m3u8_ctx(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath);
private:
virtual bool ctx_is_exist(std::string ctx);
Expand Down
22 changes: 11 additions & 11 deletions trunk/src/protocol/srs_http_stack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ srs_error_t SrsHttpFileServer::serve_mp4_file(ISrsHttpResponseWriter* w, ISrsHtt
}

// parse end in query string.
int end = -1;
long end = -1;
if (pos < range.length() - 1) {
end = ::atoi(range.substr(pos + 1).c_str());
}
Expand All @@ -524,11 +524,11 @@ srs_error_t SrsHttpFileServer::serve_mp4_file(ISrsHttpResponseWriter* w, ISrsHtt
}

return serve_mp4_stream(w, r, fullpath, start, end);
}
srs_error_t SrsHttpFileServer::serve_m3u8_file(ISrsHttpResponseWriter * w, ISrsHttpMessage * r, std::string fullpath)
}

srs_error_t SrsHttpFileServer::serve_m3u8_file(ISrsHttpResponseWriter * w, ISrsHttpMessage * r, std::string fullpath)
{
return serve_m3u8_ctx(w, r, fullpath);
return serve_m3u8_ctx(w, r, fullpath);
}

srs_error_t SrsHttpFileServer::serve_flv_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, string fullpath, int offset)
Expand All @@ -538,18 +538,18 @@ srs_error_t SrsHttpFileServer::serve_flv_stream(ISrsHttpResponseWriter* w, ISrsH
return serve_file(w, r, fullpath);
}

srs_error_t SrsHttpFileServer::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, string fullpath, int start, int end)
srs_error_t SrsHttpFileServer::serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, string fullpath, int start, long end)
{
// @remark For common http file server, we don't support stream request, please use SrsVodStream instead.
// TODO: FIXME: Support range in header https://developer.mozilla.org/zh-CN/docs/Web/HTTP/Range_requests
return serve_file(w, r, fullpath);
}
srs_error_t SrsHttpFileServer::serve_m3u8_ctx(ISrsHttpResponseWriter * w, ISrsHttpMessage * r, std::string fullpath)
{
}

srs_error_t SrsHttpFileServer::serve_m3u8_ctx(ISrsHttpResponseWriter * w, ISrsHttpMessage * r, std::string fullpath)
{
// @remark For common http file server, we don't support stream request, please use SrsVodStream instead.
// TODO: FIXME: Support range in header https://developer.mozilla.org/zh-CN/docs/Web/HTTP/Range_requests
return serve_file(w, r, fullpath);
return serve_file(w, r, fullpath);
}

srs_error_t SrsHttpFileServer::copy(ISrsHttpResponseWriter* w, SrsFileReader* fs, ISrsHttpMessage* r, int size)
Expand Down
2 changes: 1 addition & 1 deletion trunk/src/protocol/srs_http_stack.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ class SrsHttpFileServer : public ISrsHttpHandler
// @param start the start offset in bytes.
// @param end the end offset in bytes. -1 to end of file.
// @remark response data in [start, end].
virtual srs_error_t serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath, int start, int end);
virtual srs_error_t serve_mp4_stream(ISrsHttpResponseWriter* w, ISrsHttpMessage* r, std::string fullpath, int start, long end);
// For HLS protocol.
// When the request url, like as "http://127.0.0.1:8080/live/livestream.m3u8",
// returns the response like as "http://127.0.0.1:8080/live/livestream.m3u8?hls_ctx=12345678" .
Expand Down