Skip to content

Commit

Permalink
HLS: Ignore empty NALU to avoid error. v6.0.65 (ossrs#3750)
Browse files Browse the repository at this point in the history
For the DJI M30, there is a bug where empty NALU packets with a size of
zero are causing issues with HLS streaming. This bug leads to random
unpublish events due to the SRS disconnecting the connection for the HLS
module when it fails to handle empty NALU packets.

To address this bug, we have patched the system to ignore any empty NALU
packets with a size of zero. Additionally, we have created a tool in the
srs-bench to replay pcapng files captured by tcpdump or Wireshark. We
have also added utest using mprotect and asan to detect any memory
corruption.

It is important to note that this bug has been fixed in versions 4.0.271
6477f31 and 5.0.170
939f6b4. This patch specifically
addresses the issue in SRS 6.0.

Please be aware that there is another commit related to this bug that
partially fixes the issue but still leaves a small problem for asan to
detect memory corruption. This commit,
577cd29, only ignores empty NALU
packets but still reads beyond the memory.

---------

Co-authored-by: chundonglinlin <[email protected]>
  • Loading branch information
winlinvip and chundonglinlin authored Aug 2, 2023
1 parent e19efe0 commit 73dd8af
Show file tree
Hide file tree
Showing 182 changed files with 46,139 additions and 3,942 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ jobs:
uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
# Create release.
# TODO: FIXME: Refine the release when 6.0 released
# TODO: FIXME: Change prerelease to false when 6.0 released
# TODO: FIXME: Change prerelease to false and makeLatest to true when 6.0 released
- name: Update release
id: update_release
uses: ncipollo/release-action@a2e71bdd4e7dab70ca26a852f29600c98b33153e # v1.12.0
Expand Down Expand Up @@ -466,6 +466,7 @@ jobs:
* [中文FAQ](https://ossrs.net/lts/zh-cn/faq), [功能列表](https://github.com/ossrs/srs/blob/${{ github.sha }}/trunk/doc/Features.md#features) 或 [修订历史](https://github.com/ossrs/srs/blob/${{ github.sha }}/trunk/doc/CHANGELOG.md#changelog)
draft: false
prerelease: true
makeLatest: false
runs-on: ubuntu-20.04

release-done:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ jobs:
WORKDIR=$(cygpath -u $SRS_WORKSPACE) && export PATH=/usr/bin:/usr/local/bin && cd ${WORKDIR} &&
pwd && rm -rf /usr/local/srs-cache && mkdir -p /usr/local/srs-cache/srs/trunk && ls -lh &&
tar xf objs.tar.bz2 -C /usr/local/srs-cache/srs/trunk/ && du -sh /usr/local/srs-cache/srs/trunk/* &&
cd ${WORKDIR}/trunk && ./configure --gb28181=on --utest=on && ls -lh && du -sh * && du -sh objs/* &&
cd ${WORKDIR}/trunk && ./configure --h265=on --gb28181=on --utest=on && ls -lh && du -sh * && du -sh objs/* &&
cd ${WORKDIR}/trunk && make utest && ./objs/srs_utest
runs-on: windows-latest

Expand Down
3 changes: 2 additions & 1 deletion trunk/3rdparty/srs-bench/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ objs

.format.txt
.DS_Store
*.log
*.log
*.pcapng
11 changes: 10 additions & 1 deletion trunk/3rdparty/srs-bench/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ clean:
gofmt -w *.go janus
mkdir -p objs && echo "done" > ./objs/.format.bench.txt

bench: ./objs/srs_bench
bench: ./objs/srs_bench ./objs/pcap_simulator

./objs/srs_bench: ./objs/.format.bench.txt *.go janus/*.go srs/*.go vnet/*.go gb28181/*.go Makefile
go build -mod=vendor -o objs/srs_bench .
Expand All @@ -29,6 +29,15 @@ test: ./objs/srs_test ./objs/srs_gb28181_test ./objs/srs_blackbox_test
./objs/srs_test: ./objs/.format.srs.txt *.go srs/*.go vnet/*.go Makefile
go test ./srs -mod=vendor -c -o ./objs/srs_test

#########################################################################################################
# For pcap simulator test.
./objs/.format.pcap.txt: pcap/*.go
gofmt -w pcap
mkdir -p objs && echo "done" > ./objs/.format.pcap.txt

./objs/pcap_simulator: ./objs/.format.pcap.txt pcap/*.go Makefile
go build -mod=vendor -o ./objs/pcap_simulator ./pcap

#########################################################################################################
# For gb28181 test.
./objs/.format.gb28181.txt: gb28181/*.go
Expand Down
1 change: 1 addition & 0 deletions trunk/3rdparty/srs-bench/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.17

require (
github.com/ghettovoice/gosip v0.0.0-20220929080231-de8ba881be83
github.com/google/gopacket v1.1.19
github.com/ossrs/go-oryx-lib v0.0.9
github.com/pion/ice/v2 v2.3.6
github.com/pion/interceptor v0.1.17
Expand Down
5 changes: 5 additions & 0 deletions trunk/3rdparty/srs-bench/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ github.com/google/go-cmp v0.3.0/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMyw
github.com/google/go-cmp v0.3.1/go.mod h1:8QqcDgzrUqlUb/G2PQTWiueGozuR1884gddMywk6iLU=
github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/gopacket v1.1.19 h1:ves8RnFZPGiFnTS0uPQStjwru6uO6h+nlr9j6fL7kF8=
github.com/google/gopacket v1.1.19/go.mod h1:iJ8V8n6KS+z2U1A8pUwu8bW5SyEMkXJB8Yo/Vo+TKTo=
github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I=
github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU=
Expand Down Expand Up @@ -140,6 +142,8 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/crypto v0.8.0/go.mod h1:mRqEX+O9/h5TFCrQhkgjo2yKi0yYA+9ecGkdQoHrywE=
golang.org/x/crypto v0.9.0 h1:LF6fAI+IutBocDJ2OT0Q1g8plpYljMZ4+lty+dsqw3g=
golang.org/x/crypto v0.9.0/go.mod h1:yrmDGqONDYtNj3tH8X9dzUun2m2lzPa9ngI6/RUPGR0=
golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY=
golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg=
golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
Expand Down Expand Up @@ -213,6 +217,7 @@ golang.org/x/text v0.9.0 h1:2sjJmO8cDvYveuX97RDLsxlyUxLl+GHoLxBiRdHllBE=
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28=
golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
Expand Down
128 changes: 128 additions & 0 deletions trunk/3rdparty/srs-bench/pcap/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
package main

import (
"bufio"
"context"
"flag"
"fmt"
"net"
"os"
"time"

"github.com/google/gopacket"
"github.com/google/gopacket/layers"
"github.com/google/gopacket/pcapgo"
"github.com/ossrs/go-oryx-lib/errors"
"github.com/ossrs/go-oryx-lib/logger"
)

func main() {
ctx := logger.WithContext(context.Background())
if err := doMain(ctx); err != nil {
panic(err)
}
}

func trace(format string, args ...interface{}) {
fmt.Println(fmt.Sprintf(format, args...))
}

func doMain(ctx context.Context) error {
var doRE, doTrace, help bool
var pauseNumber, abortNumber uint64
var filename string
var server string
flag.BoolVar(&help, "h", false, "whether show this help")
flag.BoolVar(&help, "help", false, "whether show this help")
flag.BoolVar(&doRE, "re", true, "whether do real-time emulation")
flag.BoolVar(&doTrace, "trace", true, "whether trace the packet")
flag.Uint64Var(&pauseNumber, "pause", 0, "the packet number to pause")
flag.Uint64Var(&abortNumber, "abort", 0, "the packet number to abort")
flag.StringVar(&filename, "f", "", "the pcap filename, like ./t.pcapng")
flag.StringVar(&server, "s", "", "the server address, like 127.0.0.1:1935")

flag.Parse()

if help {
flag.Usage()
os.Exit(0)
}

if filename == "" || server == "" {
flag.Usage()
os.Exit(1)
}

logger.Tf(ctx, "Forward pcap %v to %v, re=%v, trace=%v, pause=%v, abort=%v",
filename, server, doRE, doTrace, pauseNumber, abortNumber)

f, err := os.Open(filename)
if err != nil {
return errors.Wrapf(err, "open pcap %v", filename)
}
defer f.Close()

r, err := pcapgo.NewNgReader(f, pcapgo.DefaultNgReaderOptions)
if err != nil {
return errors.Wrapf(err, "new reader")
}

// TODO: FIXME: Should start a goroutine to consume bytes from conn.
conn, err := net.Dial("tcp", server)
if err != nil {
return errors.Wrapf(err, "dial %v", server)
}
defer conn.Close()

var packetNumber uint64
var previousTime *time.Time
source := gopacket.NewPacketSource(r, r.LinkType())
for packet := range source.Packets() {
packetNumber++

if packet.Layer(layers.LayerTypeTCP) == nil {
continue
}

ci := packet.Metadata().CaptureInfo
tcp, _ := packet.Layer(layers.LayerTypeTCP).(*layers.TCP)
payload := tcp.Payload
if len(payload) == 0 {
continue
}
if tcp.DstPort != 1935 {
continue
}

if pauseNumber > 0 && packetNumber == pauseNumber {
reader := bufio.NewReader(os.Stdin)
trace("#%v Press Enter to continue...", packetNumber)
_, _ = reader.ReadString('\n')
}
if abortNumber > 0 && packetNumber > abortNumber {
break
}

if _, err := conn.Write(payload); err != nil {
return errors.Wrapf(err, "write to %v", server)
}

if doRE {
if previousTime != nil {
if diff := ci.Timestamp.Sub(*previousTime); diff > 0 {
time.Sleep(diff)
}
}
previousTime = &ci.Timestamp
}

if doTrace {
trace("#%v TCP %v=>%v %v Len:%v",
packetNumber, uint16(tcp.SrcPort), uint16(tcp.DstPort),
ci.Timestamp.Format("15:04:05.000"),
len(payload))
}
}

return nil
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 73dd8af

Please sign in to comment.