refine the protocol, add comments, add utest for empty packet

trunk/src/rtmp/srs_protocol_rtmp_stack.cpp

@@ -497,6 +497,8 @@ int SrsProtocol::do_send_message(SrsMessage* msg, SrsPacket* packet)
             }
         } else {
             // write no message header chunk stream, fmt is 3
+            // @remark, if perfer_cid > 0x3F, that is, use 2B/3B chunk header,
+            // SRS will rollback to 1B chunk header.
             *pheader++ = 0xC0 | (msg->header.perfer_cid & 0x3F);
             
             // chunk extended timestamp header, 0 or 4 bytes, big-endian
@@ -550,7 +552,8 @@ int SrsProtocol::do_send_message(SrsMessage* msg, SrsPacket* packet)
         }
     } while (p < (char*)msg->payload + msg->size);
     
-    if ((ret = on_send_message(msg, packet)) != ERROR_SUCCESS) {
+    // only process the callback event when with packet
+    if (packet && (ret = on_send_packet(msg, packet)) != ERROR_SUCCESS) {
         srs_error("hook the send message failed. ret=%d", ret);
         return ret;
     }
@@ -1224,7 +1227,7 @@ int SrsProtocol::read_message_header(SrsChunkStream* chunk, char fmt, int bh_siz
         */
         if (!is_first_chunk_of_msg && chunk_timestamp > 0 && chunk_timestamp != timestamp) {
             mh_size -= 4;
-            srs_warn("no 4bytes extended timestamp in the continued chunk");
+            srs_info("no 4bytes extended timestamp in the continued chunk");
         } else {
             chunk->header.timestamp = timestamp;
         }
@@ -1251,18 +1254,11 @@ int SrsProtocol::read_message_header(SrsChunkStream* chunk, char fmt, int bh_siz
     //        milliseconds.
     // in a word, 31bits timestamp is ok.
     // convert extended timestamp to 31bits.
-    if (chunk->header.timestamp > 0x7fffffff) {
-        srs_warn("RTMP 31bits timestamp overflow, time=%"PRId64, chunk->header.timestamp);
-    }
     chunk->header.timestamp &= 0x7fffffff;
     
-    // valid message
+    // valid message, the payload_length is 24bits,
-    if (chunk->header.payload_length < 0) {
+    // so it should never be negative.
-        ret = ERROR_RTMP_MSG_INVLIAD_SIZE;
+    srs_assert(chunk->header.payload_length >= 0);
-        srs_error("RTMP message size must not be negative. size=%d, ret=%d", 
-            chunk->header.payload_length, ret);
-        return ret;
-    }
     
     // copy header to msg
     chunk->msg->header = chunk->header;
@@ -1417,14 +1413,12 @@ int SrsProtocol::on_recv_message(SrsMessage* msg)
     return ret;
 }
 
-int SrsProtocol::on_send_message(SrsMessage* msg, SrsPacket* packet)
+int SrsProtocol::on_send_packet(SrsMessage* msg, SrsPacket* packet)
 {
     int ret = ERROR_SUCCESS;
     
-    // ignore raw bytes oriented RTMP message.
+    // should never be raw bytes oriented RTMP message.
-    if (!packet) {
+    srs_assert(packet);
-        return ret;
-    }
     
     switch (msg->header.message_type) {
         case RTMP_MSG_SetChunkSize: {

trunk/src/rtmp/srs_protocol_rtmp_stack.hpp

@@ -170,6 +170,7 @@ public:
     *       always NULL if error, 
     *       NULL for unknown packet but return success.
     *       never NULL if decode success.
+    * @remark, drop message when msg is empty or payload length is empty.
     */
     virtual int recv_message(SrsMessage** pmsg);
     /**
@@ -237,7 +238,7 @@ private:
     /**
     * when message sentout, update the context.
     */
-    virtual int on_send_message(SrsMessage* msg, SrsPacket* packet);
+    virtual int on_send_packet(SrsMessage* msg, SrsPacket* packet);
 private:
     /**
     * auto response the ack message.

trunk/src/utest/srs_utest_protocol.cpp

@@ -4425,3 +4425,41 @@ VOID TEST(ProtocolStackTest, ProtocolRecvVCid3BMax)
     EXPECT_EQ(65599, msg->header.perfer_cid);
 }
 
+/**
+* recv a zero length video message.
+*/
+VOID TEST(ProtocolStackTest, ProtocolRecvV0LenMessage)
+{
+    MockBufferIO bio;
+    SrsProtocol proto(&bio);
+    
+    // video message
+    char data[] = {
+        // video #1
+        // 12bytes header, 1byts chunk header, 11bytes msg heder
+        (char)0x03, 
+        (char)0x00, (char)0x00, (char)0x00, // timestamp
+        (char)0x00, (char)0x00, (char)0x00, // length
+        (char)0x09, // message_type
+        (char)0x00, (char)0x00, (char)0x00, (char)0x00, // stream_id
+        
+        // video #2
+        // 12bytes header, 1byts chunk header, 11bytes msg heder
+        (char)0x03, 
+        (char)0x00, (char)0x00, (char)0x00, // timestamp
+        (char)0x00, (char)0x00, (char)0x04, // length
+        (char)0x09, // message_type
+        (char)0x00, (char)0x00, (char)0x00, (char)0x00, // stream_id
+        // msg payload start
+        (char)0x00, (char)0x00, (char)0x07, (char)0x63
+    };
+    bio.in_buffer.append(data, sizeof(data));
+    
+    SrsMessage* msg = NULL;
+    ASSERT_TRUE(ERROR_SUCCESS == proto.recv_message(&msg));
+    SrsAutoFree(SrsMessage, msg);
+    EXPECT_TRUE(msg->header.is_video());
+    // protocol stack will ignore the empty video message.
+    EXPECT_EQ(4, msg->header.payload_length);
+}
+