From 999514770481bedf2aabaa44001842bf7b0016cb Mon Sep 17 00:00:00 2001 From: Winlin Date: Tue, 23 Apr 2024 15:21:36 +0800 Subject: [PATCH] RTMP: Do not response publish start message if hooks fail. v5.0.212 (#4038) Fix #4037 SRS should not send the publish start message `onStatus(NetStream.Publish.Start)` if hooks fail, which causes OBS to repeatedly reconnect. Note that this fix does not send an RTMP error message when publishing fails, because neither OBS nor FFmpeg process this specific error message; they only display a general error. Apart from the order of messages, nothing else has been changed. Previously, we sent the publish start message `onStatus(NetStream.Publish.Start)` before the HTTP hook `on_publish`; now, we have modified it to send this message after the HTTP hook. --- trunk/doc/CHANGELOG.md | 1 + trunk/src/app/srs_app_rtmp_conn.cpp | 5 +++ trunk/src/core/srs_core_version5.hpp | 2 +- .../src/protocol/srs_protocol_rtmp_stack.cpp | 37 ++++--------------- .../src/protocol/srs_protocol_rtmp_stack.hpp | 4 ++ trunk/src/utest/srs_utest_rtmp.cpp | 17 +++++++++ 6 files changed, 36 insertions(+), 30 deletions(-) diff --git a/trunk/doc/CHANGELOG.md b/trunk/doc/CHANGELOG.md index 51472ba2e..c211bb378 100644 --- a/trunk/doc/CHANGELOG.md +++ b/trunk/doc/CHANGELOG.md @@ -7,6 +7,7 @@ The changelog for SRS. ## SRS 5.0 Changelog +* v5.0, 2024-04-23, Merge [#4038](https://github.com/ossrs/srs/pull/4038): RTMP: Do not response publish start message if hooks fail. v5.0.212 (#4038) * v5.0, 2024-04-22, Merge [#4033](https://github.com/ossrs/srs/pull/4033): issue #3967: support x509 certification chiain in single pem file. v5.0.211 (#4033) * v5.0, 2024-03-26, Filter JSONP callback function name. v5.0.210 * v5.0, 2024-03-19, Merge [#3990](https://github.com/ossrs/srs/pull/3990): System: Disable feature that obtains versions and check features status. v5.0.209 (#3990) diff --git a/trunk/src/app/srs_app_rtmp_conn.cpp b/trunk/src/app/srs_app_rtmp_conn.cpp index 245ecc7aa..a17773ea8 100644 --- a/trunk/src/app/srs_app_rtmp_conn.cpp +++ b/trunk/src/app/srs_app_rtmp_conn.cpp @@ -1001,6 +1001,11 @@ srs_error_t SrsRtmpConn::do_publishing(SrsLiveSource* source, SrsPublishRecvThre ->attr("timeout", srs_fmt("%d", srsu2msi(publish_normal_timeout)))->end(); SrsAutoFree(ISrsApmSpan, span); #endif + + // Response the start publishing message, let client start to publish messages. + if ((err = rtmp->start_publishing(info->res->stream_id)) != srs_success) { + return srs_error_wrap(err, "start publishing"); + } int64_t nb_msgs = 0; uint64_t nb_frames = 0; diff --git a/trunk/src/core/srs_core_version5.hpp b/trunk/src/core/srs_core_version5.hpp index 823100817..c8909af8d 100644 --- a/trunk/src/core/srs_core_version5.hpp +++ b/trunk/src/core/srs_core_version5.hpp @@ -9,6 +9,6 @@ #define VERSION_MAJOR 5 #define VERSION_MINOR 0 -#define VERSION_REVISION 211 +#define VERSION_REVISION 212 #endif diff --git a/trunk/src/protocol/srs_protocol_rtmp_stack.cpp b/trunk/src/protocol/srs_protocol_rtmp_stack.cpp index f67b93d06..e1813fbd6 100644 --- a/trunk/src/protocol/srs_protocol_rtmp_stack.cpp +++ b/trunk/src/protocol/srs_protocol_rtmp_stack.cpp @@ -2707,20 +2707,7 @@ srs_error_t SrsRtmpServer::start_fmle_publish(int stream_id) pkt->command_name = RTMP_AMF0_COMMAND_ON_FC_PUBLISH; pkt->data->set(StatusCode, SrsAmf0Any::str(StatusCodePublishStart)); pkt->data->set(StatusDescription, SrsAmf0Any::str("Started publishing stream.")); - - if ((err = protocol->send_and_free_packet(pkt, stream_id)) != srs_success) { - return srs_error_wrap(err, "send NetStream.Publish.Start"); - } - } - // publish response onStatus(NetStream.Publish.Start) - if (true) { - SrsOnStatusCallPacket* pkt = new SrsOnStatusCallPacket(); - - pkt->data->set(StatusLevel, SrsAmf0Any::str(StatusLevelStatus)); - pkt->data->set(StatusCode, SrsAmf0Any::str(StatusCodePublishStart)); - pkt->data->set(StatusDescription, SrsAmf0Any::str("Started publishing stream.")); - pkt->data->set(StatusClientId, SrsAmf0Any::str(RTMP_SIG_CLIENT_ID)); - + if ((err = protocol->send_and_free_packet(pkt, stream_id)) != srs_success) { return srs_error_wrap(err, "send NetStream.Publish.Start"); } @@ -2758,20 +2745,6 @@ srs_error_t SrsRtmpServer::start_haivision_publish(int stream_id) } } - // publish response onStatus(NetStream.Publish.Start) - if (true) { - SrsOnStatusCallPacket* pkt = new SrsOnStatusCallPacket(); - - pkt->data->set(StatusLevel, SrsAmf0Any::str(StatusLevelStatus)); - pkt->data->set(StatusCode, SrsAmf0Any::str(StatusCodePublishStart)); - pkt->data->set(StatusDescription, SrsAmf0Any::str("Started publishing stream.")); - pkt->data->set(StatusClientId, SrsAmf0Any::str(RTMP_SIG_CLIENT_ID)); - - if ((err = protocol->send_and_free_packet(pkt, stream_id)) != srs_success) { - return srs_error_wrap(err, "send NetStream.Publish.Start"); - } - } - return err; } @@ -2818,7 +2791,13 @@ srs_error_t SrsRtmpServer::fmle_unpublish(int stream_id, double unpublish_tid) srs_error_t SrsRtmpServer::start_flash_publish(int stream_id) { srs_error_t err = srs_success; - + return err; +} + +srs_error_t SrsRtmpServer::start_publishing(int stream_id) +{ + srs_error_t err = srs_success; + // publish response onStatus(NetStream.Publish.Start) if (true) { SrsOnStatusCallPacket* pkt = new SrsOnStatusCallPacket(); diff --git a/trunk/src/protocol/srs_protocol_rtmp_stack.hpp b/trunk/src/protocol/srs_protocol_rtmp_stack.hpp index 9bd5c37a1..1abde983b 100644 --- a/trunk/src/protocol/srs_protocol_rtmp_stack.hpp +++ b/trunk/src/protocol/srs_protocol_rtmp_stack.hpp @@ -747,6 +747,10 @@ public: // When client type is publish, response with packets: // onStatus(NetStream.Publish.Start) virtual srs_error_t start_flash_publish(int stream_id); + // Response the start publishing message after hooks verified. To stop reconnecting of + // OBS when publish failed, we should never send the onStatus(NetStream.Publish.Start) + // message before failure caused by hooks. See https://github.com/ossrs/srs/issues/4037 + virtual srs_error_t start_publishing(int stream_id); public: // Expect a specified message, drop others util got specified one. // @pmsg, user must free it. NULL if not success. diff --git a/trunk/src/utest/srs_utest_rtmp.cpp b/trunk/src/utest/srs_utest_rtmp.cpp index 82b970512..049d9d261 100644 --- a/trunk/src/utest/srs_utest_rtmp.cpp +++ b/trunk/src/utest/srs_utest_rtmp.cpp @@ -1895,8 +1895,16 @@ VOID TEST(ProtocolRTMPTest, ServerFMLEStart) HELPER_ASSERT_SUCCESS(p.expect_message(&msg, &pkt)); srs_freep(msg); srs_freep(pkt); + } + + HELPER_EXPECT_SUCCESS(r.start_publishing(1)); + + if (true) { + tio.in_buffer.append(&io.out_buffer); // publish response onStatus(NetStream.Publish.Start) + SrsCommonMessage* msg = NULL; + SrsCallPacket* pkt = NULL; HELPER_ASSERT_SUCCESS(p.expect_message(&msg, &pkt)); srs_freep(msg); srs_freep(pkt); @@ -1934,8 +1942,16 @@ VOID TEST(ProtocolRTMPTest, ServerHaivisionPublish) HELPER_ASSERT_SUCCESS(p.expect_message(&msg, &pkt)); srs_freep(msg); srs_freep(pkt); + } + + HELPER_EXPECT_SUCCESS(r.start_publishing(1)); + + if (true) { + tio.in_buffer.append(&io.out_buffer); // publish response onStatus(NetStream.Publish.Start) + SrsCommonMessage* msg = NULL; + SrsCallPacket* pkt = NULL; HELPER_ASSERT_SUCCESS(p.expect_message(&msg, &pkt)); srs_freep(msg); srs_freep(pkt); @@ -2005,6 +2021,7 @@ VOID TEST(ProtocolRTMPTest, ServerFlashPublish) SrsRtmpServer r(&io); HELPER_EXPECT_SUCCESS(r.start_flash_publish(1)); + HELPER_EXPECT_SUCCESS(r.start_publishing(1)); if (true) { MockBufferIO tio;