Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crash when process rtcp feedback message. v5.0.159, v6.0.52 #3591

Merged
merged 7 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,16 @@ jobs:
steps:
- name: Download Cache for Cygwin
run: |
echo "Generate convert.sh" &&
echo "for file in \$(find objs -type l); do" > convert.sh &&
echo " REAL=\$(readlink -f \$file) &&" >> convert.sh &&
echo " echo \"convert \$file to \$REAL\" &&" >> convert.sh &&
echo " rm -rf \$file &&" >> convert.sh &&
echo " cp -r \$REAL \$file" >> convert.sh &&
echo "done" >> convert.sh &&
cat convert.sh &&
docker run --rm -v $(pwd):/srs -w /usr/local/srs-cache/srs/trunk ossrs/srs:cygwin64-cache \
tar jcf /srs/objs.tar.bz2 objs &&
bash -c "bash /srs/convert.sh && tar cf /srs/objs.tar.bz2 objs" &&
pwd && du -sh *
##################################################################################################################
- uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
Expand Down Expand Up @@ -71,7 +79,7 @@ jobs:
shell: C:\cygwin64\bin\bash.exe --login '{0}'
run: |
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 &&
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 && make utest && ./objs/srs_utest
Expand Down
2 changes: 2 additions & 0 deletions trunk/doc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ The changelog for SRS.

## SRS 6.0 Changelog

* v6.0, 2023-06-20, Merge [#3591](https://github.com/ossrs/srs/pull/3591): Fix crash when process rtcp feedback message. v6.0.52 (#3591)
* v6.0, 2023-06-15, Merge [#3581](https://github.com/ossrs/srs/pull/3581): WHIP: Add OBS support, ensuring compatibility with a unique SDP. v6.0.51 (#3581)
* v6.0, 2023-06-13, Merge [#3579](https://github.com/ossrs/srs/pull/3579): TOC: Welcome to the new TOC member, ZhangJunqin. v6.0.50 (#3579)
* v6.0, 2023-06-12, Merge [#3570](https://github.com/ossrs/srs/pull/3570): GB: Correct the range of keyframe error for compile warning. v6.0.49 (#3570)
Expand Down Expand Up @@ -65,6 +66,7 @@ The changelog for SRS.

## SRS 5.0 Changelog

* v5.0, 2023-06-20, Merge [#3591](https://github.com/ossrs/srs/pull/3591): Fix crash when process rtcp feedback message. v5.0.159 (#3591)
* v5.0, 2023-06-15, Merge [#3581](https://github.com/ossrs/srs/pull/3581): WHIP: Add OBS support, ensuring compatibility with a unique SDP. v5.0.158 (#3581)
* v5.0, 2023-06-05, Fix command injection in demonstration api-server for HTTP callback. v5.0.157
* v5.0, 2023-06-05, Merge [#3565](https://github.com/ossrs/srs/pull/3565): DTLS: Use bio callback to get fragment packet. v5.0.156 (#3565)
Expand Down
10 changes: 5 additions & 5 deletions trunk/src/app/srs_app_rtc_conn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -786,7 +786,7 @@ srs_error_t SrsRtcPlayStream::on_rtcp(SrsRtcpCommon* rtcp)
SrsRtcpNack* nack = dynamic_cast<SrsRtcpNack*>(rtcp);
return on_rtcp_nack(nack);
} else if(SrsRtcpType_psfb == rtcp->type()) {
SrsRtcpPsfbCommon* psfb = dynamic_cast<SrsRtcpPsfbCommon*>(rtcp);
SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(rtcp);
return on_rtcp_ps_feedback(psfb);
} else if(SrsRtcpType_xr == rtcp->type()) {
SrsRtcpXr* xr = dynamic_cast<SrsRtcpXr*>(rtcp);
Expand Down Expand Up @@ -866,7 +866,7 @@ srs_error_t SrsRtcPlayStream::on_rtcp_nack(SrsRtcpNack* rtcp)
return err;
}

srs_error_t SrsRtcPlayStream::on_rtcp_ps_feedback(SrsRtcpPsfbCommon* rtcp)
srs_error_t SrsRtcPlayStream::on_rtcp_ps_feedback(SrsRtcpFbCommon* rtcp)
{
srs_error_t err = srs_success;

Expand Down Expand Up @@ -2064,7 +2064,7 @@ srs_error_t SrsRtcConnection::dispatch_rtcp(SrsRtcpCommon* rtcp)

// For REMB packet.
if (SrsRtcpType_psfb == rtcp->type()) {
SrsRtcpPsfbCommon* psfb = dynamic_cast<SrsRtcpPsfbCommon*>(rtcp);
SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(rtcp);
if (15 == psfb->get_rc()) {
return on_rtcp_feedback_remb(psfb);
}
Expand Down Expand Up @@ -2092,7 +2092,7 @@ srs_error_t SrsRtcConnection::dispatch_rtcp(SrsRtcpCommon* rtcp)
required_player_ssrc = nack->get_media_ssrc();
}
} else if(SrsRtcpType_psfb == rtcp->type()) {
SrsRtcpPsfbCommon* psfb = dynamic_cast<SrsRtcpPsfbCommon*>(rtcp);
SrsRtcpFbCommon* psfb = dynamic_cast<SrsRtcpFbCommon*>(rtcp);
required_player_ssrc = psfb->get_media_ssrc();
}

Expand Down Expand Up @@ -2141,7 +2141,7 @@ srs_error_t SrsRtcConnection::on_rtcp_feedback_twcc(char* data, int nb_data)
return srs_success;
}

srs_error_t SrsRtcConnection::on_rtcp_feedback_remb(SrsRtcpPsfbCommon *rtcp)
srs_error_t SrsRtcConnection::on_rtcp_feedback_remb(SrsRtcpFbCommon *rtcp)
{
//ignore REMB
return srs_success;
Expand Down
4 changes: 2 additions & 2 deletions trunk/src/app/srs_app_rtc_conn.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ class SrsRtcPlayStream : public ISrsCoroutineHandler, public ISrsReloadHandler
private:
srs_error_t on_rtcp_xr(SrsRtcpXr* rtcp);
srs_error_t on_rtcp_nack(SrsRtcpNack* rtcp);
srs_error_t on_rtcp_ps_feedback(SrsRtcpPsfbCommon* rtcp);
srs_error_t on_rtcp_ps_feedback(SrsRtcpFbCommon* rtcp);
srs_error_t on_rtcp_rr(SrsRtcpRR* rtcp);
uint32_t get_video_publish_ssrc(uint32_t play_ssrc);
// Interface ISrsRtcPLIWorkerHandler
Expand Down Expand Up @@ -513,7 +513,7 @@ class SrsRtcConnection : public ISrsResource, public ISrsDisposingHandler, publi
srs_error_t dispatch_rtcp(SrsRtcpCommon* rtcp);
public:
srs_error_t on_rtcp_feedback_twcc(char* buf, int nb_buf);
srs_error_t on_rtcp_feedback_remb(SrsRtcpPsfbCommon *rtcp);
srs_error_t on_rtcp_feedback_remb(SrsRtcpFbCommon *rtcp);
public:
srs_error_t on_dtls_handshake_done();
srs_error_t on_dtls_alert(std::string type, std::string desc);
Expand Down
2 changes: 1 addition & 1 deletion trunk/src/core/srs_core_version5.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@

#define VERSION_MAJOR 5
#define VERSION_MINOR 0
#define VERSION_REVISION 158
#define VERSION_REVISION 159

#endif
2 changes: 1 addition & 1 deletion trunk/src/core/srs_core_version6.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@

#define VERSION_MAJOR 6
#define VERSION_MINOR 0
#define VERSION_REVISION 51
#define VERSION_REVISION 52

#endif
37 changes: 11 additions & 26 deletions trunk/src/kernel/srs_kernel_rtc_rtcp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -717,10 +717,6 @@ void SrsRtcpTWCC::clear()
next_base_sn_ = 0;
}

uint32_t SrsRtcpTWCC::get_media_ssrc() const
{
return media_ssrc_;
}
uint16_t SrsRtcpTWCC::get_base_sn() const
{
return base_sn_;
Expand All @@ -746,10 +742,6 @@ vector<uint16_t> SrsRtcpTWCC::get_recv_deltas() const
return pkt_deltas_;
}

void SrsRtcpTWCC::set_media_ssrc(uint32_t ssrc)
{
media_ssrc_ = ssrc;
}
void SrsRtcpTWCC::set_base_sn(uint16_t sn)
{
base_sn_ = sn;
Expand Down Expand Up @@ -1217,11 +1209,6 @@ SrsRtcpNack::~SrsRtcpNack()
{
}

uint32_t SrsRtcpNack::get_media_ssrc() const
{
return media_ssrc_;
}

vector<uint16_t> SrsRtcpNack::get_lost_sns() const
{
vector<uint16_t> sn;
Expand All @@ -1236,11 +1223,6 @@ bool SrsRtcpNack::empty()
return lost_sns_.empty();
}

void SrsRtcpNack::set_media_ssrc(uint32_t ssrc)
{
media_ssrc_ = ssrc;
}

void SrsRtcpNack::add_lost_sn(uint16_t sn)
{
lost_sns_.insert(sn);
Expand Down Expand Up @@ -1377,7 +1359,7 @@ srs_error_t SrsRtcpNack::encode(SrsBuffer *buffer)
return err;
}

SrsRtcpPsfbCommon::SrsRtcpPsfbCommon()
SrsRtcpFbCommon::SrsRtcpFbCommon()
{
header_.padding = 0;
header_.type = SrsRtcpType_psfb;
Expand All @@ -1386,22 +1368,22 @@ SrsRtcpPsfbCommon::SrsRtcpPsfbCommon()
//ssrc_ = sender_ssrc;
}

SrsRtcpPsfbCommon::~SrsRtcpPsfbCommon()
SrsRtcpFbCommon::~SrsRtcpFbCommon()
{

}

uint32_t SrsRtcpPsfbCommon::get_media_ssrc() const
uint32_t SrsRtcpFbCommon::get_media_ssrc() const
{
return media_ssrc_;
}

void SrsRtcpPsfbCommon::set_media_ssrc(uint32_t ssrc)
void SrsRtcpFbCommon::set_media_ssrc(uint32_t ssrc)
{
media_ssrc_ = ssrc;
}

srs_error_t SrsRtcpPsfbCommon::decode(SrsBuffer *buffer)
srs_error_t SrsRtcpFbCommon::decode(SrsBuffer *buffer)
{
/*
@doc: https://tools.ietf.org/html/rfc4585#section-6.1
Expand Down Expand Up @@ -1432,12 +1414,12 @@ srs_error_t SrsRtcpPsfbCommon::decode(SrsBuffer *buffer)
return err;
}

uint64_t SrsRtcpPsfbCommon::nb_bytes()
uint64_t SrsRtcpFbCommon::nb_bytes()
{
return kRtcpPacketSize;
}

srs_error_t SrsRtcpPsfbCommon::encode(SrsBuffer *buffer)
srs_error_t SrsRtcpFbCommon::encode(SrsBuffer *buffer)
{
return srs_error_new(ERROR_RTC_RTCP, "not support");
}
Expand Down Expand Up @@ -1762,6 +1744,9 @@ srs_error_t SrsRtcpCompound::decode(SrsBuffer *buffer)
} else if (15 == header->rc) {
//twcc
rtcp = new SrsRtcpTWCC();
} else {
// common fb
rtcp = new SrsRtcpFbCommon();
}
} else if(header->type == SrsRtcpType_psfb) {
if(1 == header->rc) {
Expand All @@ -1775,7 +1760,7 @@ srs_error_t SrsRtcpCompound::decode(SrsBuffer *buffer)
rtcp = new SrsRtcpRpsi();
} else {
// common psfb
rtcp = new SrsRtcpPsfbCommon();
rtcp = new SrsRtcpFbCommon();
}
} else if(header->type == SrsRtcpType_xr) {
rtcp = new SrsRtcpXr();
Expand Down
58 changes: 28 additions & 30 deletions trunk/src/kernel/srs_kernel_rtc_rtcp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,28 @@ class SrsRtcpRR : public SrsRtcpCommon

};

// @doc: https://tools.ietf.org/html/rfc4585#section-6.1
// As RFC 4585 says, all FB messages MUST use a common packet format,
// inlucde Transport layer FB message and Payload-specific FB message.
class SrsRtcpFbCommon : public SrsRtcpCommon
{
protected:
uint32_t media_ssrc_;
public:
SrsRtcpFbCommon();
virtual ~SrsRtcpFbCommon();

uint32_t get_media_ssrc() const;
void set_media_ssrc(uint32_t ssrc);

// interface ISrsCodec
public:
virtual srs_error_t decode(SrsBuffer *buffer);
virtual uint64_t nb_bytes();
virtual srs_error_t encode(SrsBuffer *buffer);
};


// The Message format of TWCC, @see https://tools.ietf.org/html/draft-holmer-rmcat-transport-wide-cc-extensions-01#section-3.1
// 0 1 2 3
// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
Expand Down Expand Up @@ -247,10 +269,9 @@ class SrsRtcpRR : public SrsRtcpCommon
#define kTwccFbLargeRecvDeltaBytes 2
#define kTwccFbMaxBitElements kTwccFbOneBitElements

class SrsRtcpTWCC : public SrsRtcpCommon
class SrsRtcpTWCC : public SrsRtcpFbCommon
{
private:
uint32_t media_ssrc_;
uint16_t base_sn_;
int32_t reference_time_;
uint8_t fb_pkt_count_;
Expand Down Expand Up @@ -286,14 +307,12 @@ class SrsRtcpTWCC : public SrsRtcpCommon
SrsRtcpTWCC(uint32_t sender_ssrc = 0);
virtual ~SrsRtcpTWCC();

uint32_t get_media_ssrc() const;
uint16_t get_base_sn() const;
uint32_t get_reference_time() const;
uint8_t get_feedback_count() const;
std::vector<uint16_t> get_packet_chucks() const;
std::vector<uint16_t> get_recv_deltas() const;

void set_media_ssrc(uint32_t ssrc);
void set_base_sn(uint16_t sn);
void set_reference_time(uint32_t time);
void set_feedback_count(uint8_t count);
Expand All @@ -312,7 +331,7 @@ class SrsRtcpTWCC : public SrsRtcpCommon
srs_error_t do_encode(SrsBuffer *buffer);
};

class SrsRtcpNack : public SrsRtcpCommon
class SrsRtcpNack : public SrsRtcpFbCommon
{
private:
struct SrsPidBlp {
Expand All @@ -321,17 +340,14 @@ class SrsRtcpNack : public SrsRtcpCommon
bool in_use;
};

uint32_t media_ssrc_;
std::set<uint16_t, SrsSeqCompareLess> lost_sns_;
public:
SrsRtcpNack(uint32_t sender_ssrc = 0);
virtual ~SrsRtcpNack();

uint32_t get_media_ssrc() const;
std::vector<uint16_t> get_lost_sns() const;
bool empty();

void set_media_ssrc(uint32_t ssrc);
void add_lost_sn(uint16_t sn);
// interface ISrsCodec
public:
Expand All @@ -340,25 +356,7 @@ class SrsRtcpNack : public SrsRtcpCommon
virtual srs_error_t encode(SrsBuffer *buffer);
};

class SrsRtcpPsfbCommon : public SrsRtcpCommon
{
protected:
uint32_t media_ssrc_;
public:
SrsRtcpPsfbCommon();
virtual ~SrsRtcpPsfbCommon();

uint32_t get_media_ssrc() const;
void set_media_ssrc(uint32_t ssrc);

// interface ISrsCodec
public:
virtual srs_error_t decode(SrsBuffer *buffer);
virtual uint64_t nb_bytes();
virtual srs_error_t encode(SrsBuffer *buffer);
};

class SrsRtcpPli : public SrsRtcpPsfbCommon
class SrsRtcpPli : public SrsRtcpFbCommon
{
public:
SrsRtcpPli(uint32_t sender_ssrc = 0);
Expand All @@ -371,7 +369,7 @@ class SrsRtcpPli : public SrsRtcpPsfbCommon
virtual srs_error_t encode(SrsBuffer *buffer);
};

class SrsRtcpSli : public SrsRtcpPsfbCommon
class SrsRtcpSli : public SrsRtcpFbCommon
{
private:
uint16_t first_;
Expand All @@ -388,7 +386,7 @@ class SrsRtcpSli : public SrsRtcpPsfbCommon
virtual srs_error_t encode(SrsBuffer *buffer);
};

class SrsRtcpRpsi : public SrsRtcpPsfbCommon
class SrsRtcpRpsi : public SrsRtcpFbCommon
{
private:
uint8_t pb_;
Expand All @@ -407,7 +405,7 @@ class SrsRtcpRpsi : public SrsRtcpPsfbCommon
virtual srs_error_t encode(SrsBuffer *buffer);
};

class SrsRtcpXr : public SrsRtcpCommon
class SrsRtcpXr : public SrsRtcpFbCommon
{
public:
SrsRtcpXr (uint32_t ssrc = 0);
Expand Down