[Live-devel] Invalid packets with Vorbis RTP Sink

[Gilles Chanteperdrix]

Hi,

Some time ago (actually, in 2013):
http://lists.live555.com/pipermail/live-devel/2013-December/017843.html

I reported an issue with Vorbis RTP Sink: valgrind would whine that
the sendto syscall was accessing some uninitialized data, and the
RTSP clients (vlc, using live555 for RTP depacketization, and
ffplay, using ffmpeg) would complain about invalid vorbis data when
decoding the frames received via RTP.
The file allowing to reproduce this issue being:
https://xenomai.org/video-tests/big_buck_bunny/bbb-audio.mkv

Disabling the packing of multiple vorbis frames in one packet
avoided the issue, so I did it in my local copy of live555, but this
caused issues with files containing very small frames.

So, I finally had a look at the issue, I made a test program
combining live555 media server and testRTSPClient, the server would
serve the problematic file on the 127.0.0.1 address, the RTSP client
would connect to the server and also demux the file, and compare the
frames received via RTP with the data extracted directly from the
file. I can post this test program somewhere if anyone is interested.

When the problem happens, the data extracted from the RTP packet
are (much) larger than the data extracted from the file, and the
"good data" is preceded by two bytes.

It seems the problem happens in MultiFramedRTPSink, when a frame
would overflow the packet size, and is put aside as overflow data.
When the overflow data are reused, and setFrameSpecificHeaderBytes()
is called by the vorbis doSpecialFrameHandling() method the offset
used to store the frame specific header is an offset that was valid
when the frame was at the end of the previous packet. This causes
the frame size to be larger than it should.

Recomputing the frame specific header offset as in the following
patch before reusing the overflow data seems to avoid the issue:

diff -Naurdp live/liveMedia/MultiFramedRTPSink.cpp live.fixed/liveMedia/MultiFramedRTPSink.cpp
--- live/liveMedia/MultiFramedRTPSink.cpp	2016-04-21 20:56:32.000000000 +0200
+++ live.fixed/liveMedia/MultiFramedRTPSink.cpp	2016-05-15 21:57:44.423448155 +0200
@@ -201,6 +201,10 @@ void MultiFramedRTPSink::buildAndSendPac
 void MultiFramedRTPSink::packFrame() {
   // Get the next frame.
 
+  fCurFrameSpecificHeaderPosition = fOutBuf->curPacketSize();
+  fCurFrameSpecificHeaderSize = frameSpecificHeaderSize();
+  fOutBuf->skipBytes(fCurFrameSpecificHeaderSize);
+
   // First, see if we have an overflow frame that was too big for the last pkt
   if (fOutBuf->haveOverflowData()) {
     // Use this frame before reading a new one from the source
@@ -214,9 +218,6 @@ void MultiFramedRTPSink::packFrame() {
     // Normal case: we need to read a new frame from the source
     if (fSource == NULL) return;
 
-    fCurFrameSpecificHeaderPosition = fOutBuf->curPacketSize();
-    fCurFrameSpecificHeaderSize = frameSpecificHeaderSize();
-    fOutBuf->skipBytes(fCurFrameSpecificHeaderSize);
     fTotalFrameSpecificHeaderSizes += fCurFrameSpecificHeaderSize;
 
     fSource->getNextFrame(fOutBuf->curPtr(), fOutBuf->totalBytesAvailable(),

Regards.

-- 
					    Gilles.
https://click-hack.org