mirror of
https://github.com/ossrs/srs.git
synced 2026-01-12 00:05:45 +08:00
based on @HeeJoon-Kim's patch, try to fix #4594 Fix audio-only HLS fMP4 streams causing players to skip segments. The bug was in segment_close() which always used video_dts_ (0 for audio-only) to calculate the final sample duration, causing unsigned integer overflow and corrupted m4s files. The fix passes max(audio_dts_, video_dts_) from the controller to segment_close(), ensuring correct duration calculation for both audio-only and video streams. --------- Co-authored-by: OSSRS-AI <winlinam@gmail.com>
This commit is contained in:
@@ -7,6 +7,7 @@ The changelog for SRS.
|
||||
<a name="v7-changes"></a>
|
||||
|
||||
## SRS 7.0 Changelog
|
||||
* v7.0, 2025-12-07, Merge [#4602](https://github.com/ossrs/srs/pull/4602): HLS: Fix audio-only fMP4 playback skipping. v7.0.136 (#4602)
|
||||
* v7.0, 2025-12-06, Merge [#4604](https://github.com/ossrs/srs/pull/4604): DVR: Fix HEVC mp4 recording error. v7.0.135 (#4604)
|
||||
* v7.0, 2025-12-06, SRT: Fix peer_idle_timeout not applied to publishers and players. v7.0.134 (#4600)
|
||||
* v7.0, 2025-12-04, SRT: Enable tlpktdrop by default to prevent 100% CPU usage. v7.0.133 (#4587)
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
#ifdef SRS_GB28181
|
||||
#include <srs_app_gb28181.hpp>
|
||||
#endif
|
||||
#include <srs_app_hls.hpp>
|
||||
#include <srs_app_ingest.hpp>
|
||||
#include <srs_app_listener.hpp>
|
||||
#include <srs_app_rtc_conn.hpp>
|
||||
@@ -173,6 +174,11 @@ ISrsFragmentedMp4 *SrsAppFactory::create_fragmented_mp4()
|
||||
return new SrsFragmentedMp4();
|
||||
}
|
||||
|
||||
SrsHlsM4sSegment *SrsAppFactory::create_hls_m4s_segment(ISrsFileWriter *fw)
|
||||
{
|
||||
return new SrsHlsM4sSegment(fw);
|
||||
}
|
||||
|
||||
ISrsIpListener *SrsAppFactory::create_tcp_listener(ISrsTcpHandler *handler)
|
||||
{
|
||||
return new SrsTcpListener(handler);
|
||||
|
||||
@@ -35,6 +35,7 @@ class ISrsFragment;
|
||||
class ISrsInitMp4;
|
||||
class ISrsFragmentWindow;
|
||||
class ISrsFragmentedMp4;
|
||||
class SrsHlsM4sSegment;
|
||||
class SrsFinalFactory;
|
||||
class ISrsIpListener;
|
||||
class ISrsTcpHandler;
|
||||
@@ -89,6 +90,7 @@ public:
|
||||
virtual ISrsInitMp4 *create_init_mp4() = 0;
|
||||
virtual ISrsFragmentWindow *create_fragment_window() = 0;
|
||||
virtual ISrsFragmentedMp4 *create_fragmented_mp4() = 0;
|
||||
virtual SrsHlsM4sSegment *create_hls_m4s_segment(ISrsFileWriter *fw) = 0;
|
||||
virtual ISrsIpListener *create_tcp_listener(ISrsTcpHandler *handler) = 0;
|
||||
virtual ISrsRtcConnection *create_rtc_connection(ISrsExecRtcAsyncTask *exec, const SrsContextId &cid) = 0;
|
||||
virtual ISrsFFMPEG *create_ffmpeg(std::string ffmpeg_bin) = 0;
|
||||
@@ -142,6 +144,7 @@ public:
|
||||
virtual ISrsInitMp4 *create_init_mp4();
|
||||
virtual ISrsFragmentWindow *create_fragment_window();
|
||||
virtual ISrsFragmentedMp4 *create_fragmented_mp4();
|
||||
virtual SrsHlsM4sSegment *create_hls_m4s_segment(ISrsFileWriter *fw);
|
||||
virtual ISrsIpListener *create_tcp_listener(ISrsTcpHandler *handler);
|
||||
virtual ISrsRtcConnection *create_rtc_connection(ISrsExecRtcAsyncTask *exec, const SrsContextId &cid);
|
||||
virtual ISrsFFMPEG *create_ffmpeg(std::string ffmpeg_bin);
|
||||
|
||||
@@ -200,6 +200,7 @@ srs_error_t SrsInitMp4Segment::init_encoder()
|
||||
SrsHlsM4sSegment::SrsHlsM4sSegment(ISrsFileWriter *fw)
|
||||
{
|
||||
fw_ = fw;
|
||||
sequence_no_ = 0;
|
||||
}
|
||||
|
||||
SrsHlsM4sSegment::~SrsHlsM4sSegment()
|
||||
@@ -428,7 +429,6 @@ SrsHlsFmp4Muxer::SrsHlsFmp4Muxer()
|
||||
video_track_id_ = 0;
|
||||
audio_track_id_ = 0;
|
||||
init_mp4_ready_ = false;
|
||||
video_dts_ = 0;
|
||||
|
||||
memset(key_, 0, 16);
|
||||
memset(iv_, 0, 16);
|
||||
@@ -645,8 +645,16 @@ srs_error_t SrsHlsFmp4Muxer::write_audio(SrsMediaPacket *shared_audio, SrsFormat
|
||||
}
|
||||
}
|
||||
|
||||
if (current_->duration() >= hls_fragment_) {
|
||||
if ((err = segment_close()) != srs_success) {
|
||||
// For pure audio, we use a larger threshold to reap segment.
|
||||
bool pure_audio_stream = (latest_vcodec_ == SrsVideoCodecIdForbidden || latest_vcodec_ == SrsVideoCodecIdDisabled);
|
||||
|
||||
bool reap = false;
|
||||
if (pure_audio_stream) {
|
||||
reap = is_segment_absolutely_overflow();
|
||||
}
|
||||
|
||||
if (reap) {
|
||||
if ((err = segment_close(shared_audio->timestamp_)) != srs_success) {
|
||||
return srs_error_wrap(err, "segment close");
|
||||
}
|
||||
|
||||
@@ -663,17 +671,22 @@ srs_error_t SrsHlsFmp4Muxer::write_video(SrsMediaPacket *shared_video, SrsFormat
|
||||
{
|
||||
srs_error_t err = srs_success;
|
||||
|
||||
video_dts_ = shared_video->timestamp_;
|
||||
|
||||
if (!current_) {
|
||||
if ((err = segment_open(shared_video->timestamp_ * SRS_UTIME_MILLISECONDS)) != srs_success) {
|
||||
return srs_error_wrap(err, "open segment");
|
||||
}
|
||||
}
|
||||
|
||||
bool reopen = current_->duration() >= hls_fragment_;
|
||||
bool reopen = false;
|
||||
if (is_segment_overflow()) {
|
||||
// wait for keyframe to reap segment.
|
||||
if (!wait_keyframe() || format->video_->frame_type_ == SrsVideoAvcFrameTypeKeyFrame) {
|
||||
reopen = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (reopen) {
|
||||
if ((err = segment_close()) != srs_success) {
|
||||
if ((err = segment_close(shared_video->timestamp_)) != srs_success) {
|
||||
return srs_error_wrap(err, "segment close");
|
||||
}
|
||||
|
||||
@@ -782,7 +795,7 @@ srs_error_t SrsHlsFmp4Muxer::segment_open(srs_utime_t basetime)
|
||||
}
|
||||
|
||||
// new segment.
|
||||
current_ = new SrsHlsM4sSegment(writer_);
|
||||
current_ = app_factory_->create_hls_m4s_segment(writer_);
|
||||
current_->sequence_no_ = sequence_no_++;
|
||||
|
||||
if ((err = write_hls_key()) != srs_success) {
|
||||
@@ -936,14 +949,14 @@ void SrsHlsFmp4Muxer::update_duration(uint64_t dts)
|
||||
current_->append(dts / 90);
|
||||
}
|
||||
|
||||
srs_error_t SrsHlsFmp4Muxer::segment_close()
|
||||
srs_error_t SrsHlsFmp4Muxer::segment_close(uint64_t dts)
|
||||
{
|
||||
srs_error_t err = do_segment_close();
|
||||
srs_error_t err = do_segment_close(dts);
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
srs_error_t SrsHlsFmp4Muxer::do_segment_close()
|
||||
srs_error_t SrsHlsFmp4Muxer::do_segment_close(uint64_t dts)
|
||||
{
|
||||
srs_error_t err = srs_success;
|
||||
|
||||
@@ -952,7 +965,8 @@ srs_error_t SrsHlsFmp4Muxer::do_segment_close()
|
||||
return err;
|
||||
}
|
||||
|
||||
if ((err = current_->reap(video_dts_)) != srs_success) {
|
||||
if ((err = current_->reap(dts)) != srs_success) {
|
||||
|
||||
return srs_error_wrap(err, "reap segment");
|
||||
}
|
||||
|
||||
@@ -2503,8 +2517,9 @@ srs_error_t SrsHlsMp4Controller::on_unpublish()
|
||||
srs_error_t err = srs_success;
|
||||
req_ = NULL;
|
||||
|
||||
if ((err = muxer_->segment_close()) != srs_success) {
|
||||
return srs_error_wrap(err, "hls: segment close");
|
||||
uint64_t last_dts = srs_max(audio_dts_, video_dts_);
|
||||
if ((err = muxer_->segment_close(last_dts)) != srs_success) {
|
||||
return srs_error_wrap(err, "hls: segment close");
|
||||
}
|
||||
|
||||
if ((err = muxer_->on_unpublish()) != srs_success) {
|
||||
@@ -2586,6 +2601,10 @@ srs_error_t SrsHlsMp4Controller::on_sequence_header(SrsMediaPacket *msg, SrsForm
|
||||
return srs_error_wrap(err, "write init mp4");
|
||||
}
|
||||
|
||||
if ((err = muxer_->on_sequence_header()) != srs_success) {
|
||||
return srs_error_wrap(err, "on sequence header");
|
||||
}
|
||||
|
||||
return err;
|
||||
}
|
||||
|
||||
@@ -2967,3 +2986,4 @@ void SrsHls::hls_show_mux_log()
|
||||
srsu2msi(controller_->duration()), controller_->deviation());
|
||||
}
|
||||
// LCOV_EXCL_STOP
|
||||
|
||||
|
||||
@@ -480,7 +480,6 @@ SRS_DECLARE_PRIVATE: // clang-format on
|
||||
std::string init_mp4_uri_; // URI for init.mp4 in m3u8 playlist
|
||||
int video_track_id_;
|
||||
int audio_track_id_;
|
||||
uint64_t video_dts_;
|
||||
|
||||
// clang-format off
|
||||
SRS_DECLARE_PRIVATE: // clang-format on
|
||||
@@ -559,13 +558,14 @@ public:
|
||||
// When flushing video or audio, we update the duration. But, we should also update the
|
||||
// duration before closing the segment. Keep in mind that it's fine to update the duration
|
||||
// several times using the same dts timestamp.
|
||||
void update_duration(uint64_t dts);
|
||||
// Close segment(ts).
|
||||
virtual srs_error_t segment_close();
|
||||
virtual void update_duration(uint64_t dts);
|
||||
// Close segment.
|
||||
virtual srs_error_t segment_close(uint64_t dts);
|
||||
|
||||
|
||||
// clang-format off
|
||||
SRS_DECLARE_PRIVATE: // clang-format on
|
||||
virtual srs_error_t do_segment_close();
|
||||
virtual srs_error_t do_segment_close(uint64_t dts);
|
||||
virtual srs_error_t write_hls_key();
|
||||
virtual srs_error_t refresh_m3u8();
|
||||
virtual srs_error_t do_refresh_m3u8(std::string m3u8_file);
|
||||
|
||||
@@ -9,6 +9,6 @@
|
||||
|
||||
#define VERSION_MAJOR 7
|
||||
#define VERSION_MINOR 0
|
||||
#define VERSION_REVISION 135
|
||||
#define VERSION_REVISION 136
|
||||
|
||||
#endif
|
||||
@@ -15,6 +15,7 @@
|
||||
#include <srs_app_utility.hpp>
|
||||
#include <srs_kernel_codec.hpp>
|
||||
#include <srs_kernel_error.hpp>
|
||||
#include <srs_kernel_mp4.hpp>
|
||||
#include <srs_kernel_packet.hpp>
|
||||
#include <srs_kernel_rtc_rtcp.hpp>
|
||||
#include <srs_kernel_utility.hpp>
|
||||
@@ -1416,3 +1417,88 @@ VOID TEST(HttpApiPaginationTest, ParsePagination)
|
||||
EXPECT_EQ(500, count);
|
||||
}
|
||||
}
|
||||
|
||||
// Mock SrsHlsM4sSegment to capture dts passed to reap()
|
||||
// Used for testing issue #4594
|
||||
class MockSrsHlsM4sSegmentForBug4594 : public SrsHlsM4sSegment
|
||||
{
|
||||
public:
|
||||
uint64_t captured_reap_dts_;
|
||||
bool reap_called_;
|
||||
|
||||
MockSrsHlsM4sSegmentForBug4594() : SrsHlsM4sSegment(NULL), captured_reap_dts_(0), reap_called_(false) {}
|
||||
virtual ~MockSrsHlsM4sSegmentForBug4594() {}
|
||||
|
||||
// Override reap to capture the dts parameter
|
||||
virtual srs_error_t reap(uint64_t dts) {
|
||||
reap_called_ = true;
|
||||
captured_reap_dts_ = dts;
|
||||
return srs_success;
|
||||
}
|
||||
};
|
||||
|
||||
// Mock factory to create MockSrsHlsM4sSegmentForBug4594
|
||||
class MockAppFactoryForBug4594 : public SrsAppFactory
|
||||
{
|
||||
public:
|
||||
MockSrsHlsM4sSegmentForBug4594 *mock_segment_;
|
||||
|
||||
MockAppFactoryForBug4594() : mock_segment_(NULL) {}
|
||||
virtual ~MockAppFactoryForBug4594() {}
|
||||
|
||||
virtual SrsHlsM4sSegment *create_hls_m4s_segment(ISrsFileWriter *fw) {
|
||||
mock_segment_ = new MockSrsHlsM4sSegmentForBug4594();
|
||||
return mock_segment_;
|
||||
}
|
||||
};
|
||||
|
||||
// Test for issue #4594: SrsHlsMp4Controller::on_unpublish() passes video_dts_=0 to reap()
|
||||
// for audio-only streams.
|
||||
//
|
||||
// When on_unpublish() is called for audio-only streams, the muxer's video_dts_ is 0 because
|
||||
// write_video() is never called. This causes reap(0) to be called, leading to
|
||||
// incorrect last sample duration calculation (overflow to huge values like 4294894594).
|
||||
//
|
||||
// @see https://github.com/ossrs/srs/issues/4594
|
||||
// @see https://github.com/ossrs/srs/pull/4602
|
||||
VOID TEST(HlsFmp4AudioOnlyBugTest, OnUnpublishUsesVideoDtsZeroForAudioOnly)
|
||||
{
|
||||
srs_error_t err;
|
||||
|
||||
// Create SrsHlsMp4Controller - the entry point for HLS fMP4
|
||||
SrsHlsMp4Controller controller;
|
||||
|
||||
// Access the internal muxer
|
||||
SrsHlsFmp4Muxer *muxer = controller.muxer_;
|
||||
|
||||
// Set up mock request (required by do_segment_close for async hooks)
|
||||
MockRequest mock_req("test_vhost", "live", "stream");
|
||||
muxer->req_ = &mock_req;
|
||||
|
||||
// Create mock segment that captures the dts passed to reap()
|
||||
MockSrsHlsM4sSegmentForBug4594 *mock_segment = new MockSrsHlsM4sSegmentForBug4594();
|
||||
muxer->current_ = mock_segment;
|
||||
|
||||
// Simulate audio-only stream condition:
|
||||
// - controller.audio_dts_ has a value (audio packets were written)
|
||||
// - controller.video_dts_ is 0 (no video packets, write_video() never called)
|
||||
controller.audio_dts_ = 72702; // Example audio DTS in milliseconds
|
||||
controller.video_dts_ = 0; // No video for audio-only stream
|
||||
|
||||
// Call on_unpublish() - this triggers the bug path
|
||||
HELPER_EXPECT_SUCCESS(controller.on_unpublish());
|
||||
|
||||
// Verify reap was called
|
||||
ASSERT_TRUE(mock_segment->reap_called_) << "reap() should have been called by on_unpublish()";
|
||||
|
||||
// THE BUG TEST:
|
||||
// This test SHOULD FAIL on the buggy develop branch because
|
||||
// captured_reap_dts_ will be 0 instead of a proper audio timestamp (72702)
|
||||
EXPECT_GT(mock_segment->captured_reap_dts_, 0u)
|
||||
<< "Bug #4594: on_unpublish() resulted in reap(dts=0) for audio-only stream! "
|
||||
<< "Expected non-zero dts (e.g., audio_dts_=" << controller.audio_dts_ << ") "
|
||||
<< "but reap() received " << mock_segment->captured_reap_dts_;
|
||||
|
||||
// Cleanup
|
||||
muxer->req_ = NULL;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user