ossrs / srs

SRS is a simple, high-efficiency, real-time video server supporting RTMP, WebRTC, HLS, HTTP-FLV, SRT, MPEG-DASH, and GB28181.
https://ossrs.io
MIT License
24.79k stars 5.28k forks source link

WebRTC: TCP transport should use read_fully instead of read. v5.0.194 v6.0.94 #3847

Closed chundonglinlin closed 8 months ago

chundonglinlin commented 8 months ago

SRS supports TCP WebRTC by reading 2 bytes of length, like read(buf, 2). However, in some cases, it might receive 1 byte, causing subsequent data to be incorrect and making it unable to push or play streams.


Co-authored-by: john hondaxiao@tencent.com

sandro-qiang commented 8 months ago

you do it wrong, use read_fully instead of read, you use read instead of read_fully

chundonglinlin commented 8 months ago

ou do it wrong, use read_fully instead of read, you use read instead of read_ful

Sorry, I misunderstood earlier and have made some changes. Could you please create a simple demo to test it? @sandro-qiang

TRANS_BY_GPT4

chundonglinlin commented 8 months ago

我用gpt写了个golang demo,使用TCP NODELAY选项发送单个字节的情况 测试命令:go run test.go

package main

import (
    "fmt"
    "net"
    "time"
)

func main() {
    // Set server address and port
    serverAddr := "localhost:8000"

    // connect server
    conn, err := net.Dial("tcp", serverAddr)
    if err != nil {
        fmt.Println("cannot connect server:", err)
        return
    }
    defer conn.Close()

    // Send a single byte of data
    data := []byte{0x01}
    _, err = conn.Write(data)
    if err != nil {
        fmt.Println("data send fail:", err)
        return
    }

    fmt.Println("data send success")

    time.Sleep(time.Second)

    fmt.Println("test finish.")
}
SRS调用SrsRtcTcpConn::read_packet 分支 接口 结果
bugfix/use-read-packet-for-rtc-tcp skt_->read_fully 等待后断开连接
develop skt_->read 立即读取1个字节输出
sandro-qiang commented 8 months ago

I have written a Golang demo using GPT, which sends a single byte using the TCP NODELAY option. Test command: go run test.go

package main

import (
  "fmt"
  "net"
  "time"
)

func main() {
  // Set server address and port
  serverAddr := "localhost:8000"

  // connect server
  conn, err := net.Dial("tcp", serverAddr)
  if err != nil {
      fmt.Println("cannot connect server:", err)
      return
  }
  defer conn.Close()

  // Send a single byte of data
  data := []byte{0x01}
  _, err = conn.Write(data)
  if err != nil {
      fmt.Println("data send fail:", err)
      return
  }

  fmt.Println("data send success")

  time.Sleep(time.Second)

  fmt.Println("test finish.")
}

After SRS calls SrsRtcTcpConn::read_packet

Branch Interface Result
bugfix/use-read-packet-for-rtc-tcp skt_->read_fully Waits then disconnects
develop skt_->read Immediately reads 1 byte output

This test cannot reproduce the problem, because whether you turn on no-delay or not, you only sent one byte, so the read packet length will always fail. The real problem is that at some random time, an error is reported and the connection is disconnected. The error could be that the packet length exceeds 1500, the ssrc does not exist, the packet is unknown, etc. This error is not easy to reproduce 100% through unit testing, because without no_delay, the TCP stack will decide how to divide the IP packet.

TRANS_BY_GPT4

sandro-qiang commented 8 months ago

I have written a Golang demo using GPT, which sends a single byte using the TCP NODELAY option. Test command: go run test.go

package main

import (
    "fmt"
    "net"
    "time"
)

func main() {
    // Set server address and port
    serverAddr := "localhost:8000"

    // connect server
    conn, err := net.Dial("tcp", serverAddr)
    if err != nil {
        fmt.Println("cannot connect server:", err)
        return
    }
    defer conn.Close()

    // Send a single byte of data
    data := []byte{0x01}
    _, err = conn.Write(data)
    if err != nil {
        fmt.Println("data send fail:", err)
        return
    }

    fmt.Println("data send success")

    time.Sleep(time.Second)

    fmt.Println("test finish.")
}

After SRS calls SrsRtcTcpConn::read_packet

Branch Interface Result

bugfix/use-read-packet-for-rtc-tcp skt_->read_fully Waits then disconnects

develop skt_->read Immediately reads 1 byte output

This test cannot reproduce the problem, because whether you turn on no-delay or not, you only sent one byte, so the read packet length will always fail. The real problem is that at some random time, an error is reported and the connection is disconnected. The error could be that the packet length exceeds 1500, the ssrc does not exist, the packet is unknown, etc. This error is not easy to reproduce 100% through unit testing, because without no_delay, the TCP stack will decide how to divide the IP packet.

TRANS_BY_GPT4

Perhaps you can try this: send one byte, then wait for 5 seconds, and send another byte. The srs server will print the packet length. The printed lengths in these two situations will definitely be different. Moreover, the printed length from the 'read' operation is incorrect because it only read one byte.

winlinvip commented 8 months ago

Need to merge into 5 and 6 @xiaozhihong

TRANS_BY_GPT4