Enhanced playback compatibility: Mp4Writer-generated videos now compatible with all media players

base/include/Mp4WriterSink.h

@@ -9,11 +9,12 @@ class DetailH264;
 class Mp4WriterSinkProps : public ModuleProps
 {
 public:
-	Mp4WriterSinkProps(uint32_t _chunkTime, uint32_t _syncTimeInSecs, uint16_t _fps, std::string _baseFolder, bool _recordedTSBasedDTS = true) : ModuleProps()
+	Mp4WriterSinkProps(uint32_t _chunkTime, uint32_t _syncTimeInSecs, uint16_t _fps, std::string _baseFolder, bool _recordedTSBasedDTS = true,  bool _enableMetadata = true) : ModuleProps()
 	{
 		baseFolder = _baseFolder;
 		fps = _fps;
 		recordedTSBasedDTS = _recordedTSBasedDTS;
+		enableMetadata = _enableMetadata;
 		if ((_chunkTime >= 1 && _chunkTime <= 60) || (_chunkTime == UINT32_MAX))
 		{
 			chunkTime = _chunkTime;
@@ -39,6 +40,7 @@ class Mp4WriterSinkProps : public ModuleProps
 		syncTimeInSecs = 1;
 		fps = 30;
 		recordedTSBasedDTS = true;
+		enableMetadata = true;
 	}
 
 	size_t getSerializeSize()
@@ -48,14 +50,16 @@ class Mp4WriterSinkProps : public ModuleProps
 			sizeof(baseFolder) +
 			sizeof(chunkTime) +
 			sizeof(syncTimeInSecs) +
-			sizeof(fps);
+			sizeof(fps) +
+			sizeof(enableMetadata);;
 	}
 
 	std::string baseFolder;
 	uint32_t chunkTime = 1;
 	uint32_t syncTimeInSecs = 1;
 	uint16_t fps = 30;
 	bool recordedTSBasedDTS = true;
+	bool enableMetadata = true;
 private:
 	friend class boost::serialization::access;
 
@@ -68,6 +72,7 @@ class Mp4WriterSinkProps : public ModuleProps
 		ar &chunkTime;
 		ar &syncTimeInSecs;
 		ar &fps;
+		ar &enableMetadata;
 	}
 };
 
@@ -89,6 +94,8 @@ class Mp4WriterSink : public Module
 	bool setMetadata(framemetadata_sp &inputMetadata);
 	bool handlePropsChange(frame_sp &frame);
 	bool shouldTriggerSOS();
+	void addInputPin(framemetadata_sp& metadata, string& pinId);
+	bool enableMp4Metadata(framemetadata_sp &inputMetadata);
 	boost::shared_ptr<DetailAbs> mDetail;
 	Mp4WriterSinkProps mProp;
 

base/include/Mp4WriterSinkUtils.h

@@ -13,7 +13,7 @@ class Mp4WriterSinkUtils
 	bool customNamedFileDirCheck(std::string baseFolder, uint32_t chunkTimeInMinutes, boost::filesystem::path relPath, std::string& nextFrameFileName);
 	std::string format_hrs(int &hr);
 	std::string format_2(int &min);
-	std::string filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder);
+	std::string filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder, uint64_t chunkTimeInMins);
 	~Mp4WriterSinkUtils();
 private:
 	int lastVideoMinute=0;

base/src/Mp4WriterSink.cpp

@@ -78,7 +78,7 @@ class DetailAbs
 		mNextFrameFileName = "";
 		mux = nullptr;
 		mMetadataEnabled = false;
-
+		/* DTS should be based on recorded timestamps of frames or on the fps prop entirely */
 		if (_props.recordedTSBasedDTS)
 		{
 			mDTSCalc.reset(new DTSPassThroughStrategy);
@@ -91,7 +91,7 @@ class DetailAbs
 
 	void setProps(Mp4WriterSinkProps& _props)
 	{
-		mProps.reset(new Mp4WriterSinkProps(_props.chunkTime, _props.syncTimeInSecs, _props.fps, _props.baseFolder));
+		mProps.reset(new Mp4WriterSinkProps(_props.chunkTime, _props.syncTimeInSecs, _props.fps, _props.baseFolder, _props.recordedTSBasedDTS, _props.enableMetadata));
 	}
 
 	~DetailAbs()
@@ -172,20 +172,21 @@ class DetailAbs
 			if (metatrack < 0)
 			{
 				LOG_ERROR << "Failed to add metadata track";
-				// #Dec_24_Review - should we throw exception here ? This means that user sends metadata to this module but we don't write ?
 			}
 
 			// https://www.rfc-editor.org/rfc/rfc4337.txt
 			std::string content_encoding = "base64";
 			std::string mime_format = "video/mp4";
 
-			// #Dec_24_Review - use return code from the below function call 
-			mp4_mux_track_set_metadata_mime_type(
+			auto ret = mp4_mux_track_set_metadata_mime_type(
 				mux,
 				metatrack,
 				content_encoding.c_str(),
 				mime_format.c_str());
-
+			if(ret != 0)
+			{
+				LOG_ERROR << "Failed to add metadata track mime type";
+			}
 			/* Add track reference */
 			if (metatrack > 0)
 			{
@@ -194,7 +195,6 @@ class DetailAbs
 				if (ret != 0)
 				{
 					LOG_ERROR << "Failed to add metadata track as reference";
-					// #Dec_24_Review - should we throw exception here ? This means that user sends metadata to this module but we don't write ?
 				}
 			}
 		}
@@ -280,6 +280,7 @@ class DetailH264 : public DetailAbs
 	{
 	}
 	bool write(frame_container& frames);
+	uint8_t* AppendSizeInNaluSeprator(short naluType, frame_sp frame, size_t& frameSize);
 
 	bool set_video_decoder_config()
 	{
@@ -323,7 +324,8 @@ bool DetailJpeg::write(frame_container& frames)
 
 	if (syncFlag)
 	{
-		mp4_mux_sync(mux);
+		LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">";
+		auto ret = mp4_mux_sync(mux);
 		syncFlag = false;
 	}
 
@@ -348,15 +350,78 @@ bool DetailJpeg::write(frame_container& frames)
 
 	mp4_mux_track_add_sample(mux, videotrack, &mux_sample);
 
-	if (metatrack != -1 && mMetadataEnabled && inMp4MetaFrame.get())
-	{
-		mux_sample.buffer = static_cast<uint8_t*>(inMp4MetaFrame->data());
-		mux_sample.len = inMp4MetaFrame->size();
-		mp4_mux_track_add_sample(mux, metatrack, &mux_sample);
-	}
+	if (metatrack != -1 && mMetadataEnabled)
+		{
+			if (inMp4MetaFrame.get() && inMp4MetaFrame->fIndex == 0)
+			{
+				mux_sample.buffer = static_cast<uint8_t *>(inMp4MetaFrame->data());
+				mux_sample.len = inMp4MetaFrame->size();
+			}
+			else
+			{
+				mux_sample.buffer = nullptr;
+				mux_sample.len = 0;
+			}
+
+			mp4_mux_track_add_sample(mux, metatrack, &mux_sample);
+		}
 	return true;
 }
 
+uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264ImageFrame, size_t& frameSize)
+{
+	char NaluSeprator[3] = { 00 ,00, 00 };
+	char nalusepratorIframe[2] = { 00, 00 };
+	auto nalu = reinterpret_cast<uint8_t*>(NaluSeprator);
+	uint spsPpsSize = spsBuffer.size() + ppsBuffer.size() + 8;
+	if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM)
+	{
+		frameSize = inH264ImageFrame->size();
+	}
+	//Add the size of sps and pps to I frame - (First frame of the video)
+	else if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE)
+	{
+		frameSize = inH264ImageFrame->size() + spsPpsSize;
+	}
+	uint8_t* newBuffer = new uint8_t[frameSize];
+	//add the size of sps to the 4th byte of sps's nalu seprator (00 00 00 SpsSize 67)
+	memcpy(newBuffer, nalu, 3);
+	newBuffer += 3;
+	newBuffer[0] = spsBuffer.size();
+	newBuffer += 1;
+	memcpy(newBuffer, spsBuffer.data(), spsBuffer.size());
+	newBuffer += spsBuffer.size();
+
+	//add the size of sps to the 4th byte of pps's nalu seprator (00 00 00 PpsSize 68)
+	memcpy(newBuffer, nalu, 3);
+	newBuffer += 3;
+	newBuffer[0] = ppsBuffer.size();
+	newBuffer += 1;
+	memcpy(newBuffer, ppsBuffer.data(), ppsBuffer.size());
+	newBuffer += ppsBuffer.size();
+	memcpy(newBuffer, nalusepratorIframe, 2);
+	newBuffer += 2;
+
+	//add the size of I frame to the 3rd and 4th byte of I frame's nalu seprator (00 00 frameSize frameSize 66)
+	newBuffer[0] = (frameSize - spsPpsSize - 4 >> 8) & 0xFF;
+	newBuffer[1] = frameSize - spsPpsSize - 4 & 0xFF;
+	newBuffer += 2;
+	uint8_t* tempBuffer = reinterpret_cast<uint8_t*>(inH264ImageFrame->data());
+	if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM)
+	{
+		tempBuffer = tempBuffer + spsPpsSize + 4;
+	}
+	else if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE)
+	{
+		tempBuffer = tempBuffer + 4;
+	}
+	//copy I frame data to the buffer
+	memcpy(newBuffer, tempBuffer, frameSize - spsPpsSize - 4);
+	//set the pointer to the starting of frame
+	newBuffer -= spsPpsSize + 4;
+	return newBuffer;
+}
+
 bool DetailH264::write(frame_container& frames)
 {
 	auto inH264ImageFrame = Module::getFrameByType(frames, FrameMetadata::FrameType::H264_DATA);
@@ -388,15 +453,41 @@ bool DetailH264::write(frame_container& frames)
 		return false;
 	}
 
+	uint8_t* frameData = reinterpret_cast<uint8_t*>(inH264ImageFrame->data());
+	// assign size of the frame to the last two bytes of the NALU seperator for playability in default players
+	frameData[2] = (inH264ImageFrame->size() - 4 >> 8) & 0xFF;
+	frameData[3] = inH264ImageFrame->size() - 4 & 0xFF;
+
+	mux_sample.buffer = frameData;
+	mux_sample.len = inH264ImageFrame->size();
+	auto naluType = H264Utils::getNALUType((char*)mFrameBuffer.data());
+	size_t frameSize;
 	if (mNextFrameFileName != _nextFrameFileName)
 	{
 		mNextFrameFileName = _nextFrameFileName;
 		initNewMp4File(mNextFrameFileName);
+		if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE)
+		{
+			//add sps and pps to I-frame and change the Nalu seprator according to Mp4 format
+			auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize);
+			mux_sample.buffer = newBuffer;
+			mux_sample.len = frameSize;
+		}
+	}
+
+	if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM)
+	{
+		//change the Nalu seprator according to Mp4 format
+		auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize);
+		mux_sample.buffer = newBuffer;
+		mux_sample.len = frameSize;
 	}
 
+
 	if (syncFlag)
 	{
-		mp4_mux_sync(mux);
+		LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">";
+		auto ret = mp4_mux_sync(mux);
 		syncFlag = false;
 	}
 
@@ -411,8 +502,6 @@ bool DetailH264::write(frame_container& frames)
 
 	addMetadataInVideoHeader(inH264ImageFrame);
 
-	mux_sample.buffer = static_cast<uint8_t*>(inH264ImageFrame->data());
-	mux_sample.len = inH264ImageFrame->size();
 	mux_sample.sync = isKeyFrame ? 1 : 0;
 	int64_t diffInMsecs = 0;
 
@@ -432,8 +521,16 @@ bool DetailH264::write(frame_container& frames)
 
 	if (metatrack != -1 && mMetadataEnabled && inMp4MetaFrame.get())
 	{
-		mux_sample.buffer = static_cast<uint8_t*>(inMp4MetaFrame->data());
-		mux_sample.len = inMp4MetaFrame->size();
+		if (inMp4MetaFrame->size())
+		{
+			mux_sample.buffer = static_cast<uint8_t*>(inMp4MetaFrame->data());
+			mux_sample.len = inMp4MetaFrame->size();
+		}
+		else
+		{
+			mux_sample.buffer = nullptr;
+			mux_sample.len = 0;
+		}
 		mp4_mux_track_add_sample(mux, metatrack, &mux_sample);
 	}
 	return true;
@@ -448,6 +545,8 @@ Mp4WriterSink::~Mp4WriterSink() {}
 
 bool Mp4WriterSink::init()
 {
+	bool enableVideoMetadata = false;
+	framemetadata_sp mp4VideoMetadata;
 	if (!Module::init())
 	{
 		return false;
@@ -456,7 +555,7 @@ bool Mp4WriterSink::init()
 
 	for (auto const& element : inputPinIdMetadataMap)
 	{
-		auto& metadata = element.second;
+		auto metadata = element.second;
 		auto mFrameType = metadata->getFrameType();
 		if (mFrameType == FrameMetadata::FrameType::ENCODED_IMAGE)
 		{
@@ -467,6 +566,16 @@ bool Mp4WriterSink::init()
 		{
 			mDetail.reset(new DetailH264(mProp));
 		}
+
+		else if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata)
+		{
+			enableVideoMetadata = true;
+			mp4VideoMetadata = metadata;
+		}
+	}
+	if (enableVideoMetadata)
+	{
+		enableMp4Metadata(mp4VideoMetadata);
 	}
 	return Module::init();
 }
@@ -511,11 +620,28 @@ bool Mp4WriterSink::validateInputPins()
 }
 bool Mp4WriterSink::setMetadata(framemetadata_sp& inputMetadata)
 {
-	// #Dec_24_Review - this function seems to do nothing
 	mDetail->setImageMetadata(inputMetadata);
 	return true;
 }
 
+bool Mp4WriterSink::enableMp4Metadata(framemetadata_sp &inputMetadata)
+{
+	auto mp4VideoMetadata = FrameMetadataFactory::downcast<Mp4VideoMetadata>(inputMetadata);
+	std::string formatVersion = mp4VideoMetadata->getVersion();
+	if (formatVersion.empty())
+	{
+		LOG_ERROR << "Serialization Format Information missing from the metadata. Metadata writing will be disabled";
+		return false;
+	}
+	mDetail->enableMetadata(formatVersion);
+	return true;
+}
+
+void Mp4WriterSink::addInputPin(framemetadata_sp& metadata, string& pinId)
+{
+	Module::addInputPin(metadata, pinId);
+}
+
 bool Mp4WriterSink::processSOS(frame_sp& frame)
 {
 	auto inputMetadata = frame->getMetadata();
@@ -571,10 +697,6 @@ bool Mp4WriterSink::process(frame_container& frames)
 
 bool Mp4WriterSink::processEOS(string& pinId)
 {
-	// #Dec_24_Review - generally you do opposite of what you do on SOS, so that after EOS, SOS is triggered
-	// in current state after EOS, SOS is not triggered - is it by design ? 
-	// Example EOS can be triggered if there is some resolution change in upstream module
-	// so you want to do mDetail->mInputMetadata.reset() - so that SOS gets triggered
 	return true;
 }