src/voip/h26x/h26x-utils.cpp

@@ -48,73 +48,100 @@ void H26xUtils::naluStreamToNalus(const uint8_t *bytestream, size_t size, MSQueu
 	}
 }
 
-void H26xUtils::byteStreamToNalus(const std::vector<uint8_t> &byteStream, MSQueue *out) {
-	H26xUtils::byteStreamToNalus(byteStream.data(), byteStream.size(), out);
+void H26xUtils::byteStreamToNalus(const std::vector<uint8_t> &byteStream, MSQueue *out, bool removePreventionBytes) {
+	H26xUtils::byteStreamToNalus(byteStream.data(), byteStream.size(), out, removePreventionBytes);
 }
 
-void H26xUtils::byteStreamToNalus(const uint8_t *byteStream, size_t size, MSQueue *out) {
-	vector<uint8_t> buffer;
-	const uint8_t *end = byteStream + size;
-	for (const uint8_t *it = byteStream; it != end;) {
-		buffer.resize(0);
+static bool isPictureStartCode(const uint8_t *bytestream, size_t size){
+	if (size <= 4) return false;
+	if (bytestream[0] == 0 && bytestream[1] == 0 && bytestream[2] == 0 && bytestream[3] == 1) return true;
+	return false;
+}
 
-		int leadingZero = 0;
-		while (it != end && *it == 0) {
-			leadingZero++;
-			it++;
-		}
-		if (it == end) break;
-		if (leadingZero < 2 || *it++ != 1) throw invalid_argument("no starting sequence found in H26x byte stream");
-
-		while (it != end) {
-			if (it + 2 < end && it[0] == 0 && it[1] == 0) {
-				if (it[2] == 0 || it[2] == 1) break;
-				else if (it[2] == 3) {
-					buffer.push_back(0);
-					buffer.push_back(0);
-					it += 3;
-					continue;
-				}
-			}
-			buffer.push_back(*it++);
+mblk_t * H26xUtils::makeNalu(const uint8_t *byteStream, size_t naluSize, bool removePreventionBytes, int *preventionBytesRemoved){
+	mblk_t *nalu = allocb(naluSize, 0);
+	const uint8_t *it;
+	const uint8_t *end = byteStream + naluSize;
+	for(it = byteStream; it < end ;){
+		if (removePreventionBytes && it[0] == 0 && it + 3 < end && it[1] == 0 && it[2] == 3 && it[3] == 1){
+			/* Found 0x00000301, replace by 0x000001*/
+			it += 3;
+			*nalu->b_wptr++ = 0;
+			*nalu->b_wptr++ = 0;
+			*nalu->b_wptr++ = 1;
+			(*preventionBytesRemoved)++;
+		}else{
+			*nalu->b_wptr++ = *it++;
 		}
+	}
+	return nalu;
+}
 
-		mblk_t *nalu = allocb(buffer.size(), 0);
-		memcpy(nalu->b_wptr, buffer.data(), buffer.size());
-		nalu->b_wptr += buffer.size();
-		ms_queue_put(out, nalu);
+void H26xUtils::byteStreamToNalus(const uint8_t *byteStream, size_t size, MSQueue *out, bool removePreventionBytes) {
+	int preventionBytesRemoved = 0;
+	size_t i;
+	size_t begin, end;
+	size_t naluSize;
+	
+	if (!isPictureStartCode(byteStream, size)){
+		ms_error("no picture start code found in H26x byte stream");
+		throw invalid_argument("no picutre start code found in H26x byte stream");
+		return;
+	}
+	begin = 4;
+	for (i = begin; i + 3 < size ; ++i){
+		if (byteStream[i] == 0 && byteStream[i+1] == 0 && byteStream[i+2] == 1){
+			end = i;
+			naluSize = end - begin;
+			ms_queue_put(out, makeNalu(byteStream + begin, naluSize, removePreventionBytes, &preventionBytesRemoved));
+			i+=3;
+			begin = i;
+		}
+	}
+	naluSize = size - begin;
+	ms_queue_put(out, makeNalu(byteStream + begin, naluSize, removePreventionBytes, &preventionBytesRemoved));
+	
+	if (preventionBytesRemoved > 0){
+		ms_message("Removed %i start code prevention bytes", preventionBytesRemoved);
 	}
 }
 
-void H26xUtils::nalusToByteStream(MSQueue *nalus, std::vector<uint8_t> &byteStream) {
+size_t H26xUtils::nalusToByteStream(MSQueue *nalus, uint8_t* byteStream, size_t size) {
 	bool startPicture = true;
-	byteStream.resize(0);
+	uint8_t *byteStreamEnd = byteStream + size;
+	uint8_t *it = byteStream;
+	
+	if (size < 4) throw invalid_argument("Insufficient buffer size");
+	
 	while (mblk_t *im = ms_queue_get(nalus)) {
 		if (startPicture) {
 			// starting picture extra zero byte
-			byteStream.push_back(0);
+			*it++ = 0;
 			startPicture = false;
 		}
 
 		// starting NALu marker
-		byteStream.push_back(0);
-		byteStream.push_back(0);
-		byteStream.push_back(1);
+		*it++ = 0;
+		*it++ = 0;
+		*it++ = 1;
 
 		// copy NALu content
-		for (const uint8_t *src = im->b_rptr; src < im->b_wptr;) {
-			if (src+2 < im->b_wptr && src[0] == 0 && src[1] == 0 && (src[2] == 0 || src[2] == 1)) {
-				byteStream.push_back(0);
-				byteStream.push_back(0);
-				byteStream.push_back(3); // emulation prevention three byte
-				src += 2;
+		for (const uint8_t *src = im->b_rptr; src < im->b_wptr && it < byteStreamEnd ;) {
+			if (src[0] == 0 && src+2 < im->b_wptr && src[1] == 0 && (/*src[2] == 0 ||*/ src[2] == 1)) {
+				if (it + 3 <  byteStreamEnd){
+					*it++ = 0;
+					*it++ = 0;
+					*it++ = 3; // emulation prevention three byte
+					src += 2;
+				}else throw invalid_argument("Insufficient buffer size");
 			} else {
-				byteStream.push_back(*src++);
+				*it++ = *src++;
 			}
 		}
-
 		freemsg(im);
+		if (it == byteStreamEnd) throw invalid_argument("Insufficient buffer size");
 	}
+	return it - byteStream;
 }
 
 void H26xParameterSetsInserter::replaceParameterSet(mblk_t *&ps, mblk_t *newPs) {

src/voip/h26x/h26x-utils.h

@@ -73,10 +73,14 @@ public:
 	static void naluStreamToNalus(const std::vector<uint8_t> &byteStream, MSQueue *out);
 	static void naluStreamToNalus(const uint8_t *byteStream, size_t size, MSQueue *out);
 
-	static void byteStreamToNalus(const std::vector<uint8_t> &byteStream, MSQueue *out);
-	static void byteStreamToNalus(const uint8_t *byteStream, size_t size, MSQueue *out);
+	static void byteStreamToNalus(const std::vector<uint8_t> &byteStream, MSQueue *out, bool removePreventionBytes = true);
+	static void byteStreamToNalus(const uint8_t *byteStream, size_t size, MSQueue *out, bool removePreventionBytes = true);
 
-	static void nalusToByteStream(MSQueue *nalus, std::vector<uint8_t> &byteStream);
+	/* Convert nalus to byte stream. If byteStream buffer is not large enough std::invalid_argument is thrown.*/
+	static size_t nalusToByteStream(MSQueue *nalus, uint8_t* byteStream, size_t size);
+	
+private:
+	static mblk_t * makeNalu(const uint8_t *byteStream, size_t naluSize, bool removePreventionBytes, int *preventionBytesRemoved);
 };
 
 

src/voip/h26x/media-codec-decoder.cpp

@@ -237,8 +237,6 @@ void MediaCodecDecoder::resetImpl() noexcept {
 }
 
 bool MediaCodecDecoder::feed(MSQueue *encodedFrame, uint64_t timestamp, bool isPs) {
-	H26xUtils::nalusToByteStream(encodedFrame, _bitstream);
-
 	if (_impl == nullptr) return false;
 
 	ssize_t iBufidx = AMediaCodec_dequeueInputBuffer(_impl, _timeoutUs);
@@ -248,21 +246,21 @@ bool MediaCodecDecoder::feed(MSQueue *encodedFrame, uint64_t timestamp, bool isP
 	}
 
 	size_t bufsize;
+	size_t streamSize;
 	uint8_t *buf = AMediaCodec_getInputBuffer(_impl, iBufidx, &bufsize);
 	if (buf == nullptr) {
 		ms_error("MediaCodecDecoder: AMediaCodec_getInputBuffer() returned NULL");
 		return false;
 	}
 
-	size_t size = _bitstream.size();
-	if (size > bufsize) {
-		ms_error("MediaCodecDecoder: cannot copy the all the bitstream into the input buffer size : %zu and bufsize %zu", size, bufsize);
-		size = min(size, bufsize);
+	try{
+		streamSize = H26xUtils::nalusToByteStream(encodedFrame, buf, bufsize);
+	}catch(const invalid_argument &e){
+		ms_error("MediaCodecDecoder: cannot convert the all the nal units content into the input buffer size : bufsize=%zu", bufsize);
 	}
-	memcpy(buf, _bitstream.data(), size);
 
 	uint32_t flags = isPs ? BufferFlag::CodecConfig : BufferFlag::None;
-	if (AMediaCodec_queueInputBuffer(_impl, iBufidx, 0, size, timestamp * 1000ULL, flags) != 0) {
+	if (AMediaCodec_queueInputBuffer(_impl, iBufidx, 0, streamSize, timestamp * 1000ULL, flags) != 0) {
 		ms_error("MediaCodecDecoder: AMediaCodec_queueInputBuffer() had an exception");
 		return false;
 	}

src/voip/h26x/media-codec-decoder.h

@@ -61,7 +61,6 @@ protected:
 	AMediaCodec *_impl = nullptr;
 	AMediaFormat *_format = nullptr;
 	MSYuvBufAllocator *_bufAllocator = nullptr;
-	std::vector<uint8_t> _bitstream;
 	std::unique_ptr<H26xNaluHeader> _naluHeader;
 	std::unique_ptr<H26xParameterSetsStore> _psStore;
 	int _pendingFrames = 0;

src/voip/h26x/media-codec-encoder.cpp

@@ -232,8 +232,17 @@ bool MediaCodecEncoder::fetch(MSQueue *encodedData) {
 		AMediaCodec_releaseOutputBuffer(_impl, obufidx, FALSE);
 		return false;
 	}
-
-	H26xUtils::byteStreamToNalus(buf + info.offset, info.size, &outq);
+	/*
+	 * Split bitstream into NAL units. removePreventionBytes (start code emulation prevention byte) is willingly set to false, which is not obvious.
+	 * Indeed, we should expect that the encoder has inserted prevention bytes (0x03) to escape 0x000001 start codes that could be present
+	 * within slices. When splitting into NAL units, these 0x03 bytes should be removed, since it is allowed to have 0x000001 sequences within a NAL unit.
+	 * However, the practice shows that if we do that, we get random decoding errors at the other end, whatever the other end is an Android or an iOS device.
+	 * If we do not do that, the decoding goes perfectly well. How to explain this ?
+	 * - this could mean that MediaCodec does not insert prevention bytes, but the encoder guarantees by some other means that no 0x000001 sequences can occur.
+	 *   however 0x00000301 may happen but this is not a prevention byte.
+	 * - this could mean that we have another bug elsewhere in H26xUtils routines, but then why decoding goes perfectly well when not removing prevention bytes ?
+	 */
+	H26xUtils::byteStreamToNalus(buf + info.offset, info.size, &outq, false);
 	_psInserter->process(&outq, encodedData);
 
 	AMediaCodec_releaseOutputBuffer(_impl, obufidx, FALSE);

tester/mediastreamer2_h26x_tools_tester.cpp

@@ -71,7 +71,9 @@ static void bytestream_transcoding_test(const std::vector<uint8_t> &byteStream)
 	BC_ASSERT(!ms_queue_empty(&nalus));
 
 	vector<uint8_t> byteStream2;
-	H26xUtils::nalusToByteStream(&nalus, byteStream2);
+	byteStream2.resize(byteStream.size() * 2);
+	size_t filled = H26xUtils::nalusToByteStream(&nalus, &byteStream2[0], byteStream2.size());
+	byteStream2.resize(filled);
 	BC_ASSERT(ms_queue_empty(&nalus));
 	BC_ASSERT(byteStream == byteStream2);
 
@@ -124,7 +126,9 @@ static void packing_unpacking_test(const std::vector<uint8_t> &byteStream, const
 	BC_ASSERT(!ms_queue_empty(&nalus));
 
 	vector<uint8_t> byteStream2;
-	H26xUtils::nalusToByteStream(&nalus, byteStream2);
+	byteStream2.resize(2 * byteStream.size());
+	size_t filled = H26xUtils::nalusToByteStream(&nalus, &byteStream2[0], byteStream2.size());
+	byteStream2.resize(filled);
 
 	BC_ASSERT(byteStream == byteStream2);
 }