Skip to content

Fix SIGSEGV in gst-kvs-plugin video-only pipeline when WebRTC is enabled#480

Open
narutaro wants to merge 1 commit intoaws-samples:masterfrom
narutaro:fix/video-only-webrtc-sigsegv
Open

Fix SIGSEGV in gst-kvs-plugin video-only pipeline when WebRTC is enabled#480
narutaro wants to merge 1 commit intoaws-samples:masterfrom
narutaro:fix/video-only-webrtc-sigsegv

Conversation

@narutaro
Copy link
Copy Markdown

Problem

When using kvsplugin with a video-only GStreamer pipeline (no audio pad) and connect-webrtc=true, the plugin crashes with SIGSEGV when a WebRTC viewer connects.

Two NULL pointer dereferences occur:

  1. createWebRtcStreamingSession(): pGstKvsPlugin->gstParams.audioContentType is NULL when no audio pad is connected. This NULL pointer is passed to STRNCMP(), causing a segfault.

  2. handleOffer(): receiveGstreamerAudioVideo() thread is unconditionally started, but accesses pStreamingSession->pAudioRtcRtpTransceiver which is NULL when no audio transceiver was created.

Additionally, simply skipping the audio transceiver causes the SDP answer m-line order to not match the offer, resulting in the viewer rejecting the answer.

Reproduce

gst-launch-1.0 videotestsrc is-live=true \
  ! videoconvert \
  ! x264enc bframes=0 speed-preset=veryfast bitrate=512 key-int-max=30 \
  ! h264parse \
  ! "video/x-h264,stream-format=avc,alignment=au" \
  ! kvsplugin channel-name="test-channel" connect-webrtc=true enable-streaming=false trickle-ice=true log-level=3

Connect a viewer from the KVS WebRTC test page with useTrickleICE enabled → SIGSEGV.

Fix

Change 1: NULL check for audioContentType + inactive fallback transceiver

In createWebRtcStreamingSession(), wrap the audio codec selection in a NULL check. When audioContentType is NULL (video-only pipeline), add an inactive audio transceiver with Opus codec to maintain SDP m-line order compatibility with the viewer's offer.

Change 2: Guard receiveGstreamerAudioVideo thread

In handleOffer(), only start the receiveGstreamerAudioVideo thread when pAudioRtcRtpTransceiver is not NULL.

Testing

Tested on Ubuntu EC2 (x86_64) with WebRTC SDK v1.7.3:

Scenario Result
Video-only + WebRTC only (enable-streaming=false) ✅ Viewer displays video
Video-only + WebRTC + KVS stream (enable-streaming=true) ✅ Stream saved + viewer displays video
Video + Audio (Opus) + WebRTC only ✅ Viewer displays video + audio

When using kvsplugin with a video-only GStreamer pipeline (no audio pad)
and connect-webrtc=true, the plugin crashes with SIGSEGV when a WebRTC
viewer connects due to two NULL pointer dereferences:

1. createWebRtcStreamingSession(): audioContentType is NULL when no
   audio pad is connected, causing STRNCMP to segfault. Added NULL check
   and inactive audio transceiver fallback to maintain SDP m-line order.

2. handleOffer(): receiveGstreamerAudioVideo() thread unconditionally
   accesses pAudioRtcRtpTransceiver which is NULL without audio.
   Added NULL guard to skip thread creation.
Copy link
Copy Markdown
Contributor

@sirknightj sirknightj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the contribution

audioTrack.codec = RTC_CODEC_ALAW;
} else if (STRNCMP(pGstKvsPlugin->gstParams.audioContentType, AUDIO_OPUS_CONTENT_TYPE, MAX_GSTREAMER_MEDIA_TYPE_LEN) == 0) {
audioTrack.codec = RTC_CODEC_OPUS;
// [KVS-DUAL-STREAM-TEST] Skip audio transceiver if no audio content type (video-only pipeline)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering what's up with the "dual stream test" prefix in some comments?

Copy link
Copy Markdown
Contributor

@unicornss unicornss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution!

STRCPY(audioTrack.trackId, "myAudioTrack");
RtcRtpTransceiverInit transceiverInit;
transceiverInit.direction = RTC_RTP_TRANSCEIVER_DIRECTION_INACTIVE;
CHK_STATUS(addTransceiver(pStreamingSession->pPeerConnection, &audioTrack, &transceiverInit, &pStreamingSession->pAudioRtcRtpTransceiver));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it also have the transceiverOnBandwidthEstimation ?

audioTrack.codec = RTC_CODEC_OPUS;
STRCPY(audioTrack.streamId, "myKvsVideoStream");
STRCPY(audioTrack.trackId, "myAudioTrack");
RtcRtpTransceiverInit transceiverInit;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RtcRtpTransceiverInit transceiverInit = {0}; // or memset?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants