RTMP: Do not response publish start message if hooks fail. v5.0.212 v…

…6.0.123 (#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

@@ -7,6 +7,7 @@ The changelog for SRS.
 <a name="v6-changes"></a>
 
 ## SRS 6.0 Changelog
+* v6.0, 2024-04-23, Merge [#4038](https://github.com/ossrs/srs/pull/4038): RTMP: Do not response publish start message if hooks fail. v6.0.123 (#4038)
 * v6.0, 2024-04-22, Merge [#4033](https://github.com/ossrs/srs/pull/4033): issue #3967: support x509 certification chiain in single pem file. v6.0.122 (#4033)
 * v6.0, 2024-03-26, Filter JSONP callback function name. v6.0.121
 * v6.0, 2024-03-26, Merge [#3995](https://github.com/ossrs/srs/pull/3995): Build: Refine workflow for cygwin and remove scorecard. v6.0.120 (#3995)
@@ -133,6 +134,7 @@ The changelog for SRS.
 <a name="v5-changes"></a>
 
 ## 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)

trunk/src/app/srs_app_rtmp_conn.cpp

@@ -1002,6 +1002,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;

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

trunk/src/core/srs_core_version6.hpp

@@ -9,6 +9,6 @@
 
 #define VERSION_MAJOR       6
 #define VERSION_MINOR       0
-#define VERSION_REVISION    122
+#define VERSION_REVISION    123
 
 #endif

trunk/src/protocol/srs_protocol_rtmp_stack.cpp

@@ -2705,20 +2705,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");
         }
@@ -2756,20 +2743,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;
 }
 
@@ -2816,7 +2789,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();

trunk/src/protocol/srs_protocol_rtmp_stack.hpp

@@ -747,6 +747,10 @@ class SrsRtmpServer
     // 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.

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;