From d3fce1c106f39cb2bfb42245f9f25a9e94741135 Mon Sep 17 00:00:00 2001 From: Jacob Su Date: Sun, 7 Dec 2025 11:29:12 +0800 Subject: [PATCH] HLS: Fix audio-only fMP4 playback skipping. v7.0.136 (#4602) (#4602) 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 --- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_factory.cpp | 6 ++ trunk/src/app/srs_app_factory.hpp | 3 + trunk/src/app/srs_app_hls.cpp | 48 +++++++++++----- trunk/src/app/srs_app_hls.hpp | 10 ++-- trunk/src/core/srs_core_version7.hpp | 2 +- trunk/src/utest/srs_utest_ai24.cpp | 86 ++++++++++++++++++++++++++++ 7 files changed, 136 insertions(+), 20 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index c23f61892..8cb3dd64b 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ The changelog for SRS. ## 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) diff --git a/trunk/src/app/srs_app_factory.cpp b/trunk/src/app/srs_app_factory.cpp index 6819006e0..7f3af4be3 100644 --- a/trunk/src/app/srs_app_factory.cpp +++ b/trunk/src/app/srs_app_factory.cpp @@ -15,6 +15,7 @@ #ifdef SRS_GB28181 #include #endif +#include #include #include #include @@ -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); diff --git a/trunk/src/app/srs_app_factory.hpp b/trunk/src/app/srs_app_factory.hpp index 4bd7c44c7..7129dae42 100644 --- a/trunk/src/app/srs_app_factory.hpp +++ b/trunk/src/app/srs_app_factory.hpp @@ -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); diff --git a/trunk/src/app/srs_app_hls.cpp b/trunk/src/app/srs_app_hls.cpp index 6c5572697..8efe091d6 100644 --- a/trunk/src/app/srs_app_hls.cpp +++ b/trunk/src/app/srs_app_hls.cpp @@ -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 + diff --git a/trunk/src/app/srs_app_hls.hpp b/trunk/src/app/srs_app_hls.hpp index df66a8d2f..9883217f3 100644 --- a/trunk/src/app/srs_app_hls.hpp +++ b/trunk/src/app/srs_app_hls.hpp @@ -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); diff --git a/trunk/src/core/srs_core_version7.hpp b/trunk/src/core/srs_core_version7.hpp index 48a1d2fc3..c24bdbf73 100644 --- a/trunk/src/core/srs_core_version7.hpp +++ b/trunk/src/core/srs_core_version7.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 7 #define VERSION_MINOR 0 -#define VERSION_REVISION 135 +#define VERSION_REVISION 136 #endif \ No newline at end of file diff --git a/trunk/src/utest/srs_utest_ai24.cpp b/trunk/src/utest/srs_utest_ai24.cpp index ba6cad3ad..e8e53ac0e 100644 --- a/trunk/src/utest/srs_utest_ai24.cpp +++ b/trunk/src/utest/srs_utest_ai24.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -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; +}