From 5ff94b876e9afc191e9d88d11d7704e8b7ace92d Mon Sep 17 00:00:00 2001 From: zaki Date: Wed, 28 Jun 2023 14:52:55 +0530 Subject: [PATCH 01/10] Bug fix: Videos now playable on all players written using mp4Writer --- base/include/Mp4WriterSink.h | 11 ++- base/include/Mp4WriterSinkUtils.h | 2 +- base/src/Mp4WriterSink.cpp | 147 ++++++++++++++++++++++++++---- base/src/Mp4WriterSinkUtils.cpp | 22 +++-- base/test/mp4writersink_tests.cpp | 10 +- 5 files changed, 160 insertions(+), 32 deletions(-) diff --git a/base/include/Mp4WriterSink.h b/base/include/Mp4WriterSink.h index ed6e47f0a..8bcff4dba 100644 --- a/base/include/Mp4WriterSink.h +++ b/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,7 +50,8 @@ class Mp4WriterSinkProps : public ModuleProps sizeof(baseFolder) + sizeof(chunkTime) + sizeof(syncTimeInSecs) + - sizeof(fps); + sizeof(fps) + + sizeof(enableMetadata);; } std::string baseFolder; @@ -56,6 +59,7 @@ class Mp4WriterSinkProps : public ModuleProps 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 mDetail; Mp4WriterSinkProps mProp; diff --git a/base/include/Mp4WriterSinkUtils.h b/base/include/Mp4WriterSinkUtils.h index 32507e85f..55b253a71 100644 --- a/base/include/Mp4WriterSinkUtils.h +++ b/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; diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index ac932cdc9..1345609bd 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -77,8 +77,7 @@ class DetailAbs setProps(_props); 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 +90,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() @@ -280,6 +279,8 @@ class DetailH264 : public DetailAbs { } bool write(frame_container& frames); + uint8_t* prependSizeBeforeNaluSeparatorIFrame(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,70 @@ 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(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 % 3 == 0) + { + mux_sample.buffer = static_cast(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::prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) +{ + char NaluSeprator[3] = { 00 ,00, 00 }; + char nalusepratorIframe[2] = { 00, 00 }; + auto nalu = reinterpret_cast(NaluSeprator); + uint spsPpsSize = spsBuffer.size() + ppsBuffer.size() + 8; + if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) + { + frameSize = inH264ImageFrame->size(); + } + else if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE) + { + frameSize = inH264ImageFrame->size() + spsPpsSize; + } + uint8_t* newBuffer = new uint8_t[frameSize]; + memcpy(newBuffer, nalu, 3); + newBuffer += 3; + newBuffer[0] = spsBuffer.size(); + newBuffer += 1; + memcpy(newBuffer, spsBuffer.data(), spsBuffer.size()); + newBuffer += spsBuffer.size(); + 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; + newBuffer[0] = (frameSize - spsPpsSize - 4 >> 8) & 0xFF; + newBuffer[1] = frameSize - spsPpsSize - 4 & 0xFF; + newBuffer += 2; + uint8_t* tempBuffer = reinterpret_cast(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; + } + memcpy(newBuffer, tempBuffer, frameSize - spsPpsSize - 4); + newBuffer -= spsPpsSize + 4; + return newBuffer; +} + bool DetailH264::write(frame_container& frames) { auto inH264ImageFrame = Module::getFrameByType(frames, FrameMetadata::FrameType::H264_DATA); @@ -388,15 +445,40 @@ bool DetailH264::write(frame_container& frames) return false; } + uint8_t* frameData = reinterpret_cast(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) + { + auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + mux_sample.buffer = newBuffer; + mux_sample.len = frameSize; + } } + + if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) + { + + auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(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 +493,6 @@ bool DetailH264::write(frame_container& frames) addMetadataInVideoHeader(inH264ImageFrame); - mux_sample.buffer = static_cast(inH264ImageFrame->data()); - mux_sample.len = inH264ImageFrame->size(); mux_sample.sync = isKeyFrame ? 1 : 0; int64_t diffInMsecs = 0; @@ -432,8 +512,16 @@ bool DetailH264::write(frame_container& frames) if (metatrack != -1 && mMetadataEnabled && inMp4MetaFrame.get()) { - mux_sample.buffer = static_cast(inMp4MetaFrame->data()); - mux_sample.len = inMp4MetaFrame->size(); + if (inMp4MetaFrame.get()) + { + mux_sample.buffer = static_cast(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; @@ -456,7 +544,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) { @@ -468,6 +556,15 @@ bool Mp4WriterSink::init() mDetail.reset(new DetailH264(mProp)); } } + for (auto const& element : inputPinIdMetadataMap) + { + auto metadata = element.second; + auto mFrameType = metadata->getFrameType(); + if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata) + { + enableMp4Metadata(metadata); + } + } return Module::init(); } @@ -516,6 +613,24 @@ bool Mp4WriterSink::setMetadata(framemetadata_sp& inputMetadata) return true; } +bool Mp4WriterSink::enableMp4Metadata(framemetadata_sp &inputMetadata) +{ + auto mp4VideoMetadata = FrameMetadataFactory::downcast(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(); diff --git a/base/src/Mp4WriterSinkUtils.cpp b/base/src/Mp4WriterSinkUtils.cpp index be3fa5bb8..2430b31f9 100644 --- a/base/src/Mp4WriterSinkUtils.cpp +++ b/base/src/Mp4WriterSinkUtils.cpp @@ -36,9 +36,15 @@ std::string Mp4WriterSinkUtils::format_2(int &num) } } -std::string Mp4WriterSinkUtils::filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder) +std::string Mp4WriterSinkUtils::filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder, uint64_t chunkTimeInMinutes) { boost::filesystem::path finalPath; + std::string mp4VideoPath; + if (customNamedFileDirCheck(baseFolder, chunkTimeInMinutes, relPath, mp4VideoPath)) + { + return mp4VideoPath; + } + auto folderPath = boost::filesystem::path(baseFolder) / relPath; if (boost::filesystem::is_directory(folderPath)) { @@ -121,11 +127,11 @@ void Mp4WriterSinkUtils::parseTSJpeg(uint64_t &ts, uint32_t &chunkTimeInMinutes, // used cached values if the difference in ts is less than chunkTime uint32_t chunkTimeInSecs = 60 * chunkTimeInMinutes; - if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder && chunkTimeInMinutes != UINT32_MAX) + if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder) { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); return; } @@ -144,7 +150,7 @@ void Mp4WriterSinkUtils::parseTSJpeg(uint64_t &ts, uint32_t &chunkTimeInMinutes, lastVideoName = mp4FileName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); } void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, uint32_t& syncTimeInSeconds,boost::filesystem::path& relPath, std::string& mp4FileName, bool& syncFlag, short frameType, short naluType, std::string baseFolder, std::string& nextFrameFileName) @@ -169,15 +175,15 @@ void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, } // used cached values if the difference in ts is less than chunkTime uint32_t chunkTimeInSecs = 60 * chunkTimeInMinutes; - if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder && chunkTimeInMinutes != UINT32_MAX) + if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder)// && chunkTimeInMinutes != UINT32_MAX { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); return; } // cannot be merged with if condition above. - if (naluType != H264Utils::H264_NAL_TYPE::H264_NAL_TYPE_IDR_SLICE && chunkTimeInMinutes != UINT32_MAX) + if (naluType != H264Utils::H264_NAL_TYPE::H264_NAL_TYPE_IDR_SLICE && naluType != H264Utils::H264_NAL_TYPE_SEQ_PARAM) { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; @@ -200,7 +206,7 @@ void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, lastVideoName = mp4FileName; lastVideoFolderPath = relPath; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); tempNextFrameFileName = nextFrameFileName; } diff --git a/base/test/mp4writersink_tests.cpp b/base/test/mp4writersink_tests.cpp index e85497b31..07d4b4807 100644 --- a/base/test/mp4writersink_tests.cpp +++ b/base/test/mp4writersink_tests.cpp @@ -29,7 +29,7 @@ void writeH264(bool readLoop, int sleepSeconds, std::string outFolderPath, int c Logger::initLogger(loggerProps); auto fileReaderProps = FileReaderModuleProps(inFolderPath, 0, -1); - fileReaderProps.fps = 100; + fileReaderProps.fps = 24; fileReaderProps.readLoop = readLoop; auto fileReader = boost::shared_ptr(new FileReaderModule(fileReaderProps)); @@ -295,14 +295,14 @@ BOOST_AUTO_TEST_CASE(setgetprops_jpeg) LOG_ERROR << "processing folder <" << inFolderPath << ">"; p->run_all_threaded(); - Test_Utils::sleep_for_seconds(20); + Test_Utils::sleep_for_seconds(10); Mp4WriterSinkProps propschange = mp4WriterSink->getProps(); propschange.chunkTime = 2; propschange.baseFolder = changedOutFolderPath; mp4WriterSink->setProps(propschange); - Test_Utils::sleep_for_seconds(70); + Test_Utils::sleep_for_seconds(10); p->stop(); p->term(); @@ -498,7 +498,7 @@ BOOST_AUTO_TEST_CASE(setgetprops_h264) propschange.baseFolder = changedOutFolderPath; mp4WriterSink->setProps(propschange); - Test_Utils::sleep_for_seconds(60); + Test_Utils::sleep_for_seconds(20); p->stop(); p->term(); @@ -523,7 +523,7 @@ BOOST_AUTO_TEST_CASE(single_file_given_name_h264) // custom name is only supported while writing to single video file (chunktime = UINT32_MAX). std::string outFolderPath = "./data/testOutput/mp4_videos/h264_videos/apraH264.mp4"; - writeH264(true,80,outFolderPath, UINT32_MAX); + writeH264(true,10,outFolderPath, UINT32_MAX); } BOOST_AUTO_TEST_CASE(read_mul_write_one_as_recorded) From 5293fa608d8abb0aa12164a781d1ead538cb1b0e Mon Sep 17 00:00:00 2001 From: zaki Date: Wed, 28 Jun 2023 15:02:40 +0530 Subject: [PATCH 02/10] changed function name --- base/src/Mp4WriterSink.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 1345609bd..7791572e6 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -279,7 +279,7 @@ class DetailH264 : public DetailAbs { } bool write(frame_container& frames); - uint8_t* prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp frame, size_t& frameSize); + uint8_t* AppendSizeInNaluSeprator(short naluType, frame_sp frame, size_t& frameSize); bool set_video_decoder_config() @@ -368,7 +368,7 @@ bool DetailJpeg::write(frame_container& frames) return true; } -uint8_t* DetailH264::prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) +uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) { char NaluSeprator[3] = { 00 ,00, 00 }; char nalusepratorIframe[2] = { 00, 00 }; @@ -460,7 +460,7 @@ bool DetailH264::write(frame_container& frames) initNewMp4File(mNextFrameFileName); if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE) { - auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize); mux_sample.buffer = newBuffer; mux_sample.len = frameSize; } @@ -469,7 +469,7 @@ bool DetailH264::write(frame_container& frames) if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) { - auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize); mux_sample.buffer = newBuffer; mux_sample.len = frameSize; } From af0a0a99de9ea183681a890e598f656a7fbf950c Mon Sep 17 00:00:00 2001 From: zaki Date: Tue, 11 Jul 2023 19:04:38 +0530 Subject: [PATCH 03/10] Addressed PR comments --- base/src/Mp4WriterSink.cpp | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 7791572e6..ecc6eaa6e 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -171,20 +171,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) { @@ -193,7 +194,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 ? } } } @@ -352,7 +352,7 @@ bool DetailJpeg::write(frame_container& frames) if (metatrack != -1 && mMetadataEnabled) { - if (inMp4MetaFrame.get() && inMp4MetaFrame->fIndex % 3 == 0) + if (inMp4MetaFrame.get() && inMp4MetaFrame->fIndex == 0) { mux_sample.buffer = static_cast(inMp4MetaFrame->data()); mux_sample.len = inMp4MetaFrame->size(); @@ -378,17 +378,21 @@ uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264Ima { 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(); @@ -397,6 +401,8 @@ uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264Ima 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; @@ -409,7 +415,9 @@ uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264Ima { 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; } @@ -460,6 +468,7 @@ bool DetailH264::write(frame_container& frames) 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; @@ -468,7 +477,7 @@ bool DetailH264::write(frame_container& frames) 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; @@ -608,7 +617,6 @@ bool Mp4WriterSink::validateInputPins() } bool Mp4WriterSink::setMetadata(framemetadata_sp& inputMetadata) { - // #Dec_24_Review - this function seems to do nothing mDetail->setImageMetadata(inputMetadata); return true; } @@ -686,10 +694,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; } From 95ea3dc6d8461d4a5f15423083f9b2560cb0a498 Mon Sep 17 00:00:00 2001 From: zaki Date: Wed, 28 Jun 2023 14:52:55 +0530 Subject: [PATCH 04/10] Bug fix: Videos now playable on all players written using mp4Writer --- base/include/Mp4WriterSink.h | 11 ++- base/include/Mp4WriterSinkUtils.h | 2 +- base/src/Mp4WriterSink.cpp | 147 ++++++++++++++++++++++++++---- base/src/Mp4WriterSinkUtils.cpp | 22 +++-- base/test/mp4writersink_tests.cpp | 10 +- 5 files changed, 160 insertions(+), 32 deletions(-) diff --git a/base/include/Mp4WriterSink.h b/base/include/Mp4WriterSink.h index ed6e47f0a..8bcff4dba 100644 --- a/base/include/Mp4WriterSink.h +++ b/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,7 +50,8 @@ class Mp4WriterSinkProps : public ModuleProps sizeof(baseFolder) + sizeof(chunkTime) + sizeof(syncTimeInSecs) + - sizeof(fps); + sizeof(fps) + + sizeof(enableMetadata);; } std::string baseFolder; @@ -56,6 +59,7 @@ class Mp4WriterSinkProps : public ModuleProps 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 mDetail; Mp4WriterSinkProps mProp; diff --git a/base/include/Mp4WriterSinkUtils.h b/base/include/Mp4WriterSinkUtils.h index 32507e85f..55b253a71 100644 --- a/base/include/Mp4WriterSinkUtils.h +++ b/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; diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index ac932cdc9..1345609bd 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -77,8 +77,7 @@ class DetailAbs setProps(_props); 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 +90,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() @@ -280,6 +279,8 @@ class DetailH264 : public DetailAbs { } bool write(frame_container& frames); + uint8_t* prependSizeBeforeNaluSeparatorIFrame(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,70 @@ 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(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 % 3 == 0) + { + mux_sample.buffer = static_cast(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::prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) +{ + char NaluSeprator[3] = { 00 ,00, 00 }; + char nalusepratorIframe[2] = { 00, 00 }; + auto nalu = reinterpret_cast(NaluSeprator); + uint spsPpsSize = spsBuffer.size() + ppsBuffer.size() + 8; + if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) + { + frameSize = inH264ImageFrame->size(); + } + else if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE) + { + frameSize = inH264ImageFrame->size() + spsPpsSize; + } + uint8_t* newBuffer = new uint8_t[frameSize]; + memcpy(newBuffer, nalu, 3); + newBuffer += 3; + newBuffer[0] = spsBuffer.size(); + newBuffer += 1; + memcpy(newBuffer, spsBuffer.data(), spsBuffer.size()); + newBuffer += spsBuffer.size(); + 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; + newBuffer[0] = (frameSize - spsPpsSize - 4 >> 8) & 0xFF; + newBuffer[1] = frameSize - spsPpsSize - 4 & 0xFF; + newBuffer += 2; + uint8_t* tempBuffer = reinterpret_cast(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; + } + memcpy(newBuffer, tempBuffer, frameSize - spsPpsSize - 4); + newBuffer -= spsPpsSize + 4; + return newBuffer; +} + bool DetailH264::write(frame_container& frames) { auto inH264ImageFrame = Module::getFrameByType(frames, FrameMetadata::FrameType::H264_DATA); @@ -388,15 +445,40 @@ bool DetailH264::write(frame_container& frames) return false; } + uint8_t* frameData = reinterpret_cast(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) + { + auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + mux_sample.buffer = newBuffer; + mux_sample.len = frameSize; + } } + + if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) + { + + auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(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 +493,6 @@ bool DetailH264::write(frame_container& frames) addMetadataInVideoHeader(inH264ImageFrame); - mux_sample.buffer = static_cast(inH264ImageFrame->data()); - mux_sample.len = inH264ImageFrame->size(); mux_sample.sync = isKeyFrame ? 1 : 0; int64_t diffInMsecs = 0; @@ -432,8 +512,16 @@ bool DetailH264::write(frame_container& frames) if (metatrack != -1 && mMetadataEnabled && inMp4MetaFrame.get()) { - mux_sample.buffer = static_cast(inMp4MetaFrame->data()); - mux_sample.len = inMp4MetaFrame->size(); + if (inMp4MetaFrame.get()) + { + mux_sample.buffer = static_cast(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; @@ -456,7 +544,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) { @@ -468,6 +556,15 @@ bool Mp4WriterSink::init() mDetail.reset(new DetailH264(mProp)); } } + for (auto const& element : inputPinIdMetadataMap) + { + auto metadata = element.second; + auto mFrameType = metadata->getFrameType(); + if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata) + { + enableMp4Metadata(metadata); + } + } return Module::init(); } @@ -516,6 +613,24 @@ bool Mp4WriterSink::setMetadata(framemetadata_sp& inputMetadata) return true; } +bool Mp4WriterSink::enableMp4Metadata(framemetadata_sp &inputMetadata) +{ + auto mp4VideoMetadata = FrameMetadataFactory::downcast(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(); diff --git a/base/src/Mp4WriterSinkUtils.cpp b/base/src/Mp4WriterSinkUtils.cpp index be3fa5bb8..2430b31f9 100644 --- a/base/src/Mp4WriterSinkUtils.cpp +++ b/base/src/Mp4WriterSinkUtils.cpp @@ -36,9 +36,15 @@ std::string Mp4WriterSinkUtils::format_2(int &num) } } -std::string Mp4WriterSinkUtils::filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder) +std::string Mp4WriterSinkUtils::filePath(boost::filesystem::path relPath, std::string mp4FileName, std::string baseFolder, uint64_t chunkTimeInMinutes) { boost::filesystem::path finalPath; + std::string mp4VideoPath; + if (customNamedFileDirCheck(baseFolder, chunkTimeInMinutes, relPath, mp4VideoPath)) + { + return mp4VideoPath; + } + auto folderPath = boost::filesystem::path(baseFolder) / relPath; if (boost::filesystem::is_directory(folderPath)) { @@ -121,11 +127,11 @@ void Mp4WriterSinkUtils::parseTSJpeg(uint64_t &ts, uint32_t &chunkTimeInMinutes, // used cached values if the difference in ts is less than chunkTime uint32_t chunkTimeInSecs = 60 * chunkTimeInMinutes; - if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder && chunkTimeInMinutes != UINT32_MAX) + if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder) { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); return; } @@ -144,7 +150,7 @@ void Mp4WriterSinkUtils::parseTSJpeg(uint64_t &ts, uint32_t &chunkTimeInMinutes, lastVideoName = mp4FileName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); } void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, uint32_t& syncTimeInSeconds,boost::filesystem::path& relPath, std::string& mp4FileName, bool& syncFlag, short frameType, short naluType, std::string baseFolder, std::string& nextFrameFileName) @@ -169,15 +175,15 @@ void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, } // used cached values if the difference in ts is less than chunkTime uint32_t chunkTimeInSecs = 60 * chunkTimeInMinutes; - if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder && chunkTimeInMinutes != UINT32_MAX) + if ((t - lastVideoTS) < chunkTimeInSecs && currentFolder == baseFolder)// && chunkTimeInMinutes != UINT32_MAX { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); return; } // cannot be merged with if condition above. - if (naluType != H264Utils::H264_NAL_TYPE::H264_NAL_TYPE_IDR_SLICE && chunkTimeInMinutes != UINT32_MAX) + if (naluType != H264Utils::H264_NAL_TYPE::H264_NAL_TYPE_IDR_SLICE && naluType != H264Utils::H264_NAL_TYPE_SEQ_PARAM) { relPath = lastVideoFolderPath; mp4FileName = lastVideoName; @@ -200,7 +206,7 @@ void Mp4WriterSinkUtils::parseTSH264(uint64_t& ts, uint32_t& chunkTimeInMinutes, lastVideoName = mp4FileName; lastVideoFolderPath = relPath; - nextFrameFileName = filePath(relPath, mp4FileName, baseFolder); + nextFrameFileName = filePath(relPath, mp4FileName, baseFolder, chunkTimeInMinutes); tempNextFrameFileName = nextFrameFileName; } diff --git a/base/test/mp4writersink_tests.cpp b/base/test/mp4writersink_tests.cpp index e85497b31..07d4b4807 100644 --- a/base/test/mp4writersink_tests.cpp +++ b/base/test/mp4writersink_tests.cpp @@ -29,7 +29,7 @@ void writeH264(bool readLoop, int sleepSeconds, std::string outFolderPath, int c Logger::initLogger(loggerProps); auto fileReaderProps = FileReaderModuleProps(inFolderPath, 0, -1); - fileReaderProps.fps = 100; + fileReaderProps.fps = 24; fileReaderProps.readLoop = readLoop; auto fileReader = boost::shared_ptr(new FileReaderModule(fileReaderProps)); @@ -295,14 +295,14 @@ BOOST_AUTO_TEST_CASE(setgetprops_jpeg) LOG_ERROR << "processing folder <" << inFolderPath << ">"; p->run_all_threaded(); - Test_Utils::sleep_for_seconds(20); + Test_Utils::sleep_for_seconds(10); Mp4WriterSinkProps propschange = mp4WriterSink->getProps(); propschange.chunkTime = 2; propschange.baseFolder = changedOutFolderPath; mp4WriterSink->setProps(propschange); - Test_Utils::sleep_for_seconds(70); + Test_Utils::sleep_for_seconds(10); p->stop(); p->term(); @@ -498,7 +498,7 @@ BOOST_AUTO_TEST_CASE(setgetprops_h264) propschange.baseFolder = changedOutFolderPath; mp4WriterSink->setProps(propschange); - Test_Utils::sleep_for_seconds(60); + Test_Utils::sleep_for_seconds(20); p->stop(); p->term(); @@ -523,7 +523,7 @@ BOOST_AUTO_TEST_CASE(single_file_given_name_h264) // custom name is only supported while writing to single video file (chunktime = UINT32_MAX). std::string outFolderPath = "./data/testOutput/mp4_videos/h264_videos/apraH264.mp4"; - writeH264(true,80,outFolderPath, UINT32_MAX); + writeH264(true,10,outFolderPath, UINT32_MAX); } BOOST_AUTO_TEST_CASE(read_mul_write_one_as_recorded) From e4cf54a040e92eff470f84a162172d8c414957db Mon Sep 17 00:00:00 2001 From: zaki Date: Wed, 28 Jun 2023 15:02:40 +0530 Subject: [PATCH 05/10] changed function name --- base/src/Mp4WriterSink.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 1345609bd..7791572e6 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -279,7 +279,7 @@ class DetailH264 : public DetailAbs { } bool write(frame_container& frames); - uint8_t* prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp frame, size_t& frameSize); + uint8_t* AppendSizeInNaluSeprator(short naluType, frame_sp frame, size_t& frameSize); bool set_video_decoder_config() @@ -368,7 +368,7 @@ bool DetailJpeg::write(frame_container& frames) return true; } -uint8_t* DetailH264::prependSizeBeforeNaluSeparatorIFrame(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) +uint8_t* DetailH264::AppendSizeInNaluSeprator(short naluType, frame_sp inH264ImageFrame, size_t& frameSize) { char NaluSeprator[3] = { 00 ,00, 00 }; char nalusepratorIframe[2] = { 00, 00 }; @@ -460,7 +460,7 @@ bool DetailH264::write(frame_container& frames) initNewMp4File(mNextFrameFileName); if (naluType == H264Utils::H264_NAL_TYPE_IDR_SLICE) { - auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize); mux_sample.buffer = newBuffer; mux_sample.len = frameSize; } @@ -469,7 +469,7 @@ bool DetailH264::write(frame_container& frames) if (naluType == H264Utils::H264_NAL_TYPE_SEQ_PARAM) { - auto newBuffer = prependSizeBeforeNaluSeparatorIFrame(naluType, inH264ImageFrame, frameSize); + auto newBuffer = AppendSizeInNaluSeprator(naluType, inH264ImageFrame, frameSize); mux_sample.buffer = newBuffer; mux_sample.len = frameSize; } From ca6c039e14c63f13539916adb99fd56205e67bd8 Mon Sep 17 00:00:00 2001 From: zaki Date: Thu, 13 Jul 2023 12:36:03 +0530 Subject: [PATCH 06/10] Adressed PR comments and updated vcpkg baseline --- base/src/Mp4WriterSink.cpp | 5 +- base/test/mp4writersink_tests.cpp | 138 +++++++++++++++++++++++++++++- base/vcpkg.json | 2 +- vcpkg | 2 +- 4 files changed, 141 insertions(+), 6 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index ecc6eaa6e..5cd65b4f7 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -461,7 +461,7 @@ bool DetailH264::write(frame_container& frames) mux_sample.buffer = frameData; mux_sample.len = inH264ImageFrame->size(); auto naluType = H264Utils::getNALUType((char*)mFrameBuffer.data()); - size_t frameSize;; + size_t frameSize; if (mNextFrameFileName != _nextFrameFileName) { mNextFrameFileName = _nextFrameFileName; @@ -521,7 +521,7 @@ bool DetailH264::write(frame_container& frames) if (metatrack != -1 && mMetadataEnabled && inMp4MetaFrame.get()) { - if (inMp4MetaFrame.get()) + if (inMp4MetaFrame->size()) { mux_sample.buffer = static_cast(inMp4MetaFrame->data()); mux_sample.len = inMp4MetaFrame->size(); @@ -565,6 +565,7 @@ bool Mp4WriterSink::init() mDetail.reset(new DetailH264(mProp)); } } + //two loops because - enableMp4Metadata will fail if metadata module is added first in pipeline followed by H264 or Jpeg metadata for (auto const& element : inputPinIdMetadataMap) { auto metadata = element.second; diff --git a/base/test/mp4writersink_tests.cpp b/base/test/mp4writersink_tests.cpp index 07d4b4807..ab3a26716 100644 --- a/base/test/mp4writersink_tests.cpp +++ b/base/test/mp4writersink_tests.cpp @@ -13,6 +13,8 @@ #include "H264Utils.h" #include "ExternalSinkModule.h" #include "Mp4ReaderSource.h" +#include "ExternalSourceModule.h" +#include BOOST_AUTO_TEST_SUITE(mp4WriterSink_tests) @@ -62,6 +64,8 @@ void writeH264(bool readLoop, int sleepSeconds, std::string outFolderPath, int c p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); } void write(std::string inFolderPath, std::string outFolderPath, int width, int height, int chunkTime = 1) { @@ -104,6 +108,8 @@ void write(std::string inFolderPath, std::string outFolderPath, int width, int h p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); } void write_metadata(std::string inFolderPath, std::string outFolderPath, std::string metadataPath, int width, int height, int fps) @@ -160,6 +166,8 @@ void write_metadata(std::string inFolderPath, std::string outFolderPath, std::st p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); } void read_write(std::string videoPath, std::string outPath, int width, int height,framemetadata_sp metadata, @@ -201,10 +209,10 @@ void read_write(std::string videoPath, std::string outPath, int width, int heigh p->stop(); p->term(); - p->wait_for_all(); - p.reset(); + + Test_Utils::deleteFolder(outPath); } BOOST_AUTO_TEST_CASE(jpg_rgb_24_to_mp4v) @@ -308,6 +316,9 @@ BOOST_AUTO_TEST_CASE(setgetprops_jpeg) p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); + Test_Utils::deleteFolder(changedOutFolderPath); } BOOST_AUTO_TEST_CASE(h264_to_mp4v) @@ -383,6 +394,8 @@ BOOST_AUTO_TEST_CASE(h264_metadata, *boost::unit_test::disabled()) p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); } BOOST_AUTO_TEST_CASE(parsenalu) @@ -504,6 +517,9 @@ BOOST_AUTO_TEST_CASE(setgetprops_h264) p->term(); p->wait_for_all(); p.reset(); + + Test_Utils::deleteFolder(outFolderPath); + Test_Utils::deleteFolder(changedOutFolderPath); } BOOST_AUTO_TEST_CASE(single_file_given_name_jpeg) @@ -540,4 +556,122 @@ BOOST_AUTO_TEST_CASE(read_mul_write_one_as_recorded) read_write(videoPath, outPath, 704, 576, h264ImageMetadata, recordedTSBasedDTS, parseFS, UINT32_MAX); } +BOOST_AUTO_TEST_CASE(write_mp4video_h264_step) +{ + std::string inFolderPath = "./data/h264_data"; + std::string metadataPath = "./data/Metadata/"; + std::string outPath = "data/testOutput/mp4_videos/h264/step/stepvideo.mp4"; + int width = 704; + int height = 576; + + LoggerProps loggerProps; + loggerProps.logLevel = boost::log::trivial::severity_level::info; + Logger::setLogLevel(boost::log::trivial::severity_level::info); + Logger::initLogger(loggerProps); + + auto fileReaderProps = FileReaderModuleProps(inFolderPath, 0, -1); + fileReaderProps.fps = 24; + fileReaderProps.readLoop = false; + + auto fileReader = boost::shared_ptr(new FileReaderModule(fileReaderProps)); + auto h264ImageMetadata = framemetadata_sp(new H264Metadata(width, height)); + fileReader->addOutputPin(h264ImageMetadata); + + auto mp4WriterSinkProps = Mp4WriterSinkProps(UINT32_MAX, 10, 100, outPath); + mp4WriterSinkProps.logHealth = true; + mp4WriterSinkProps.logHealthFrequency = 100; + auto mp4WriterSink = boost::shared_ptr(new Mp4WriterSink(mp4WriterSinkProps)); + fileReader->setNext(mp4WriterSink); + + fileReader->play(true); + + BOOST_TEST(fileReader->init()); + BOOST_TEST(mp4WriterSink->init()); + + for (int i = 0; i < 230; i++) + { + fileReader->step(); + mp4WriterSink->step(); + } + mp4WriterSink->term(); + boost::filesystem::path mp4fileName = "data/testOutput/mp4_videos/h264/step/stepvideo.mp4"; + auto fileSize = boost::filesystem::file_size(mp4fileName); + BOOST_CHECK_CLOSE(fileSize, 4268040.0, 5000.0); + + Test_Utils::deleteFolder(mp4fileName.string()); + +} + +BOOST_AUTO_TEST_CASE(write_mp4video_metadata_h264_step) +{ + std::string videoMetadata = "aprapipes"; + std::string inFolderPath = "./data/h264_data"; + std::string outPath = "data/testOutput/mp4_videos/h264_metadata/step/stepvideo.mp4"; + int width = 704; + int height = 576; + + LoggerProps loggerProps; + loggerProps.logLevel = boost::log::trivial::severity_level::info; + Logger::setLogLevel(boost::log::trivial::severity_level::info); + Logger::initLogger(loggerProps); + + auto fileReaderProps = FileReaderModuleProps(inFolderPath, 0, -1); + fileReaderProps.fps = 24; + fileReaderProps.readLoop = true; + + auto fileReader = boost::shared_ptr(new FileReaderModule(fileReaderProps)); + auto encodedImageMetadata = framemetadata_sp(new H264Metadata(width, height)); + fileReader->addOutputPin(encodedImageMetadata); + + + auto metadataSource = boost::shared_ptr(new ExternalSourceModule()); + + auto mp4Metadata = framemetadata_sp(new Mp4VideoMetadata("v_1_0")); + auto metadataPinId = metadataSource->addOutputPin(mp4Metadata); + + FramesMuxerProps muxerProps; + muxerProps.strategy = FramesMuxerProps::MAX_DELAY_ANY; + muxerProps.maxDelay = 0; + auto readerMuxer = boost::shared_ptr(new FramesMuxer(muxerProps)); + fileReader->setNext(readerMuxer); + metadataSource->setNext(readerMuxer); + + auto mp4WriterSinkProps = Mp4WriterSinkProps(UINT32_MAX, 10, fileReaderProps.fps, outPath); + mp4WriterSinkProps.logHealth = true; + mp4WriterSinkProps.logHealthFrequency = 100; + auto mp4WriterSink = boost::shared_ptr(new Mp4WriterSink(mp4WriterSinkProps)); + readerMuxer->setNext(mp4WriterSink); + + fileReader->play(true); + + BOOST_TEST(fileReader->init()); + BOOST_TEST(metadataSource->init()); + BOOST_TEST(readerMuxer->init()); + BOOST_TEST(mp4WriterSink->init()); + for (int i = 0; i < 230; i++) + { + fileReader->step(); + auto metaFrame = metadataSource->makeFrame(0, metadataPinId); + if (i % 6 == 0) + { + metaFrame = metadataSource->makeFrame(videoMetadata.size(), metadataPinId); + memcpy(metaFrame->data(), videoMetadata.data(), videoMetadata.size()); + } + std::chrono::time_point t = std::chrono::system_clock::now(); + auto dur = std::chrono::duration_cast(t.time_since_epoch()); + metaFrame->timestamp = dur.count(); + frame_container frames; + frames.insert(make_pair(metadataPinId, metaFrame)); + metadataSource->send(frames); + readerMuxer->step(); + readerMuxer->step(); + mp4WriterSink->step(); + } + mp4WriterSink->term(); + boost::filesystem::path mp4FileName = "data/testOutput/mp4_videos/h264_metadata/step/stepvideo.mp4"; + auto fileSize = boost::filesystem::file_size(mp4FileName); + BOOST_CHECK_CLOSE(fileSize, 4268040.0, 5000.0); + + Test_Utils::deleteFolder(mp4FileName.string()); +} BOOST_AUTO_TEST_SUITE_END() diff --git a/base/vcpkg.json b/base/vcpkg.json index d68a86bb6..1759fa668 100644 --- a/base/vcpkg.json +++ b/base/vcpkg.json @@ -2,7 +2,7 @@ "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", "name": "apra-pipes-cuda", "version": "0.0.1", - "builtin-baseline": "b0053dfa8bbaabd1cfc602b27d3cf7295987a073", + "builtin-baseline": "36bfee6dd9d131b6d73e741f99d9a07def9fb4cb", "dependencies": [ { "name": "opencv4", diff --git a/vcpkg b/vcpkg index b0053dfa8..36bfee6dd 160000 --- a/vcpkg +++ b/vcpkg @@ -1 +1 @@ -Subproject commit b0053dfa8bbaabd1cfc602b27d3cf7295987a073 +Subproject commit 36bfee6dd9d131b6d73e741f99d9a07def9fb4cb From dbabb9604b3e6630adf4427ad1ecfba85b09fc88 Mon Sep 17 00:00:00 2001 From: zaki Date: Thu, 13 Jul 2023 15:05:29 +0530 Subject: [PATCH 07/10] Updated vcpkg baseline commit --- base/src/Mp4WriterSink.cpp | 26 -------------------------- base/vcpkg.json | 2 +- vcpkg | 2 +- 3 files changed, 2 insertions(+), 28 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 1ae2d9d0b..766568d19 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -282,9 +282,6 @@ class DetailH264 : public DetailAbs bool write(frame_container& frames); uint8_t* AppendSizeInNaluSeprator(short naluType, frame_sp frame, size_t& frameSize); - uint8_t* AppendSizeInNaluSeprator(short naluType, frame_sp frame, size_t& frameSize); - - bool set_video_decoder_config() { vdc.width = mWidth; @@ -327,8 +324,6 @@ bool DetailJpeg::write(frame_container& frames) if (syncFlag) { - LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">"; - auto ret = mp4_mux_sync(mux); LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">"; auto ret = mp4_mux_sync(mux); syncFlag = false; @@ -491,8 +486,6 @@ bool DetailH264::write(frame_container& frames) if (syncFlag) { - LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">"; - auto ret = mp4_mux_sync(mux); LOG_TRACE << "attempting to sync <" << mNextFrameFileName << ">"; auto ret = mp4_mux_sync(mux); syncFlag = false; @@ -560,7 +553,6 @@ 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) @@ -648,24 +640,6 @@ void Mp4WriterSink::addInputPin(framemetadata_sp& metadata, string& pinId) Module::addInputPin(metadata, pinId); } -bool Mp4WriterSink::enableMp4Metadata(framemetadata_sp &inputMetadata) -{ - auto mp4VideoMetadata = FrameMetadataFactory::downcast(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(); diff --git a/base/vcpkg.json b/base/vcpkg.json index 1759fa668..a93199c9b 100644 --- a/base/vcpkg.json +++ b/base/vcpkg.json @@ -2,7 +2,7 @@ "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", "name": "apra-pipes-cuda", "version": "0.0.1", - "builtin-baseline": "36bfee6dd9d131b6d73e741f99d9a07def9fb4cb", + "builtin-baseline": "8c349b21332b3c2c05ae986f23d82431dcaae7e7", "dependencies": [ { "name": "opencv4", diff --git a/vcpkg b/vcpkg index 36bfee6dd..8c349b213 160000 --- a/vcpkg +++ b/vcpkg @@ -1 +1 @@ -Subproject commit 36bfee6dd9d131b6d73e741f99d9a07def9fb4cb +Subproject commit 8c349b21332b3c2c05ae986f23d82431dcaae7e7 From 1d57d4dc6f34bd02b2484d610bd6d77b6794a420 Mon Sep 17 00:00:00 2001 From: zaki Date: Fri, 14 Jul 2023 16:06:30 +0530 Subject: [PATCH 08/10] Addressed PR comments --- base/src/Mp4WriterSink.cpp | 18 ++++++++++-------- base/test/mp4writersink_tests.cpp | 11 +++++++++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 766568d19..55d31851d 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -545,6 +545,8 @@ Mp4WriterSink::~Mp4WriterSink() {} bool Mp4WriterSink::init() { + bool enableVideoMetadata = false; + framemetadata_sp Mp4VideoMetadata; if (!Module::init()) { return false; @@ -564,17 +566,17 @@ bool Mp4WriterSink::init() { mDetail.reset(new DetailH264(mProp)); } - } - //two loops because - enableMp4Metadata will fail if metadata module is added first in pipeline followed by H264 or Jpeg metadata - for (auto const& element : inputPinIdMetadataMap) - { - auto metadata = element.second; - auto mFrameType = metadata->getFrameType(); - if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata) + + else if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata) { - enableMp4Metadata(metadata); + enableVideoMetadata = true; + Mp4VideoMetadata = metadata; } } + if (enableVideoMetadata) + { + enableMp4Metadata(Mp4VideoMetadata); + } return Module::init(); } diff --git a/base/test/mp4writersink_tests.cpp b/base/test/mp4writersink_tests.cpp index ab3a26716..16c9d59b9 100644 --- a/base/test/mp4writersink_tests.cpp +++ b/base/test/mp4writersink_tests.cpp @@ -593,10 +593,12 @@ BOOST_AUTO_TEST_CASE(write_mp4video_h264_step) fileReader->step(); mp4WriterSink->step(); } + fileReader->term(); mp4WriterSink->term(); boost::filesystem::path mp4fileName = "data/testOutput/mp4_videos/h264/step/stepvideo.mp4"; auto fileSize = boost::filesystem::file_size(mp4fileName); - BOOST_CHECK_CLOSE(fileSize, 4268040.0, 5000.0); + //checking the size of mp4 file + BOOST_TEST(fileSize, 4270314); Test_Utils::deleteFolder(mp4fileName.string()); @@ -654,9 +656,11 @@ BOOST_AUTO_TEST_CASE(write_mp4video_metadata_h264_step) auto metaFrame = metadataSource->makeFrame(0, metadataPinId); if (i % 6 == 0) { + //write metadata to only every 6th frame . For every other frame the metadata size will be 0. All the 230 frames with 230 entries for metadata as well metaFrame = metadataSource->makeFrame(videoMetadata.size(), metadataPinId); memcpy(metaFrame->data(), videoMetadata.data(), videoMetadata.size()); } + //writing the current timestamp to the file . std::chrono::time_point t = std::chrono::system_clock::now(); auto dur = std::chrono::duration_cast(t.time_since_epoch()); metaFrame->timestamp = dur.count(); @@ -667,10 +671,13 @@ BOOST_AUTO_TEST_CASE(write_mp4video_metadata_h264_step) readerMuxer->step(); mp4WriterSink->step(); } + fileReader->term(); + metadataSource->term(); + readerMuxer->term(); mp4WriterSink->term(); boost::filesystem::path mp4FileName = "data/testOutput/mp4_videos/h264_metadata/step/stepvideo.mp4"; auto fileSize = boost::filesystem::file_size(mp4FileName); - BOOST_CHECK_CLOSE(fileSize, 4268040.0, 5000.0); + BOOST_TEST(fileSize == 4270665); Test_Utils::deleteFolder(mp4FileName.string()); } From 9705555425dd07cc7217d18236969ad94516c606 Mon Sep 17 00:00:00 2001 From: zaki Date: Fri, 14 Jul 2023 20:00:03 +0530 Subject: [PATCH 09/10] addressed PR comments --- base/src/Mp4WriterSink.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/base/src/Mp4WriterSink.cpp b/base/src/Mp4WriterSink.cpp index 55d31851d..23abe46fc 100644 --- a/base/src/Mp4WriterSink.cpp +++ b/base/src/Mp4WriterSink.cpp @@ -77,7 +77,7 @@ class DetailAbs setProps(_props); mNextFrameFileName = ""; mux = nullptr; - /* DTS should be based on recorded timestamps of frames or on the fps prop entirely */ + mMetadataEnabled = false; /* DTS should be based on recorded timestamps of frames or on the fps prop entirely */ if (_props.recordedTSBasedDTS) { @@ -546,7 +546,7 @@ Mp4WriterSink::~Mp4WriterSink() {} bool Mp4WriterSink::init() { bool enableVideoMetadata = false; - framemetadata_sp Mp4VideoMetadata; + framemetadata_sp mp4VideoMetadata; if (!Module::init()) { return false; @@ -570,12 +570,12 @@ bool Mp4WriterSink::init() else if (mFrameType == FrameMetadata::FrameType::MP4_VIDEO_METADATA && mProp.enableMetadata) { enableVideoMetadata = true; - Mp4VideoMetadata = metadata; + mp4VideoMetadata = metadata; } } if (enableVideoMetadata) { - enableMp4Metadata(Mp4VideoMetadata); + enableMp4Metadata(mp4VideoMetadata); } return Module::init(); } From 873856aadcaedb82cf4792ccfb7bc13e3192b5e5 Mon Sep 17 00:00:00 2001 From: zaki Date: Mon, 17 Jul 2023 18:15:32 +0530 Subject: [PATCH 10/10] updated vcpkg.json baseline --- base/vcpkg.json | 2 +- vcpkg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/base/vcpkg.json b/base/vcpkg.json index a93199c9b..0b44d4233 100644 --- a/base/vcpkg.json +++ b/base/vcpkg.json @@ -2,7 +2,7 @@ "$schema": "https://raw.githubusercontent.com/microsoft/vcpkg/master/scripts/vcpkg.schema.json", "name": "apra-pipes-cuda", "version": "0.0.1", - "builtin-baseline": "8c349b21332b3c2c05ae986f23d82431dcaae7e7", + "builtin-baseline": "b02012c71b988dd3f4de0a3cd40b468b6fab79c2", "dependencies": [ { "name": "opencv4", diff --git a/vcpkg b/vcpkg index 8c349b213..b02012c71 160000 --- a/vcpkg +++ b/vcpkg @@ -1 +1 @@ -Subproject commit 8c349b21332b3c2c05ae986f23d82431dcaae7e7 +Subproject commit b02012c71b988dd3f4de0a3cd40b468b6fab79c2