Browse Source

Fix the parts pointed out in the code review (#1127).

melpon 4 months ago
parent
commit
414f9af48c

+ 9 - 4
include/rtc/dependencydescriptor.hpp

@@ -77,16 +77,21 @@ struct DependencyDescriptor {
 	bool structureAttached;
 	bool structureAttached;
 };
 };
 
 
+struct DependencyDescriptorContext {
+	DependencyDescriptor descriptor;
+	std::bitset<32> activeChains;
+	FrameDependencyStructure structure;
+};
+
 // Write dependency descriptor to RTP Header Extension
 // Write dependency descriptor to RTP Header Extension
 // Dependency descriptor specification is here:
 // Dependency descriptor specification is here:
 // https://aomediacodec.github.io/av1-rtp-spec/#dependency-descriptor-rtp-header-extension
 // https://aomediacodec.github.io/av1-rtp-spec/#dependency-descriptor-rtp-header-extension
 class DependencyDescriptorWriter {
 class DependencyDescriptorWriter {
 public:
 public:
-	DependencyDescriptorWriter(const FrameDependencyStructure &structure,
-	                           std::bitset<32> activeChains,
-	                           const DependencyDescriptor &descriptor);
+	explicit DependencyDescriptorWriter(const DependencyDescriptorContext& context);
 	size_t getSizeBits() const;
 	size_t getSizeBits() const;
-	void writeTo(std::byte *buf, size_t sizeBits) const;
+	size_t getSize() const;
+	void writeTo(std::byte *buf, size_t sizeBytes) const;
 
 
 private:
 private:
 	void doWriteTo(BitWriter &writer) const;
 	void doWriteTo(BitWriter &writer) const;

+ 0 - 5
include/rtc/rtppacketizationconfig.hpp

@@ -64,11 +64,6 @@ public:
 
 
 	// Dependency Descriptor Extension Header
 	// Dependency Descriptor Extension Header
 	uint8_t dependencyDescriptorId = 0;
 	uint8_t dependencyDescriptorId = 0;
-	struct DependencyDescriptorContext {
-		DependencyDescriptor descriptor;
-		std::bitset<32> activeChains;
-		FrameDependencyStructure structure;
-	};
 
 
 	optional<DependencyDescriptorContext> dependencyDescriptorContext;
 	optional<DependencyDescriptorContext> dependencyDescriptorContext;
 	// the negotiated ID of the playout delay header extension
 	// the negotiated ID of the playout delay header extension

+ 7 - 6
src/dependencydescriptor.cpp

@@ -204,19 +204,20 @@ static bool find_best_template(const FrameDependencyStructure &structure,
 
 
 static const uint32_t MaxTemplates = 64;
 static const uint32_t MaxTemplates = 64;
 
 
-DependencyDescriptorWriter::DependencyDescriptorWriter(const FrameDependencyStructure &structure,
-                                                       std::bitset<32> activeChains,
-                                                       const DependencyDescriptor &descriptor)
-    : mStructure(structure), mActiveChains(activeChains), mDescriptor(descriptor) {}
+DependencyDescriptorWriter::DependencyDescriptorWriter(const DependencyDescriptorContext& context)
+	: mStructure(context.structure), mActiveChains(context.activeChains), mDescriptor(context.descriptor) {}
 
 
 size_t DependencyDescriptorWriter::getSizeBits() const {
 size_t DependencyDescriptorWriter::getSizeBits() const {
 	auto writer = rtc::BitWriter::fromNull();
 	auto writer = rtc::BitWriter::fromNull();
 	doWriteTo(writer);
 	doWriteTo(writer);
 	return writer.getWrittenBits();
 	return writer.getWrittenBits();
 }
 }
+size_t DependencyDescriptorWriter::getSize() const {
+	return (getSizeBits() + 7) / 8;
+}
 
 
-void DependencyDescriptorWriter::writeTo(std::byte *buf, size_t sizeBits) const {
-	auto writer = BitWriter::fromSizeBits(buf, 0, (sizeBits + 7) / 8 * 8);
+void DependencyDescriptorWriter::writeTo(std::byte *buf, size_t sizeBytes) const {
+	auto writer = BitWriter::fromSizeBits(buf, 0, sizeBytes * 8);
 	doWriteTo(writer);
 	doWriteTo(writer);
 	// Pad up to the byte boundary
 	// Pad up to the byte boundary
 	if (auto bits = (writer.getWrittenBits() % 8); bits != 0) {
 	if (auto bits = (writer.getWrittenBits() % 8); bits != 0) {

+ 14 - 16
src/rtppacketizer.cpp

@@ -31,12 +31,15 @@ message_ptr RtpPacketizer::packetize(const binary &payload, bool mark) {
 	const bool setVideoRotation =
 	const bool setVideoRotation =
 	    (rtpConfig->videoOrientationId != 0) && mark && (rtpConfig->videoOrientation != 0);
 	    (rtpConfig->videoOrientationId != 0) && mark && (rtpConfig->videoOrientation != 0);
 
 
+	std::optional<DependencyDescriptorWriter> ddWriter;
+	if (rtpConfig->dependencyDescriptorContext.has_value()) {
+		ddWriter.emplace(*rtpConfig->dependencyDescriptorContext);
+	}
+
 	// Determine if a two-byte header is necessary
 	// Determine if a two-byte header is necessary
 	// Check for dependency descriptor extension
 	// Check for dependency descriptor extension
-	if (rtpConfig->dependencyDescriptorContext.has_value()) {
-		auto &ctx = *rtpConfig->dependencyDescriptorContext;
-		DependencyDescriptorWriter writer(ctx.structure, ctx.activeChains, ctx.descriptor);
-		auto sizeBytes = (writer.getSizeBits() + 7) / 8;
+	if (ddWriter.has_value()) {
+		auto sizeBytes = ddWriter->getSize();
 		if (sizeBytes > 16 || rtpConfig->dependencyDescriptorId > 14) {
 		if (sizeBytes > 16 || rtpConfig->dependencyDescriptorId > 14) {
 			twoByteHeader = true;
 			twoByteHeader = true;
 		}
 		}
@@ -63,10 +66,8 @@ message_ptr RtpPacketizer::packetize(const binary &payload, bool mark) {
 	if (rtpConfig->rid.has_value())
 	if (rtpConfig->rid.has_value())
 		rtpExtHeaderSize += headerSize + rtpConfig->rid->length();
 		rtpExtHeaderSize += headerSize + rtpConfig->rid->length();
 
 
-	if (rtpConfig->dependencyDescriptorContext.has_value()) {
-		auto &ctx = *rtpConfig->dependencyDescriptorContext;
-		DependencyDescriptorWriter writer(ctx.structure, ctx.activeChains, ctx.descriptor);
-		rtpExtHeaderSize += headerSize + (writer.getSizeBits() + 7) / 8;
+	if (ddWriter.has_value()) {
+		rtpExtHeaderSize += headerSize + ddWriter->getSize();
 	}
 	}
 
 
 	if (rtpExtHeaderSize != 0)
 	if (rtpExtHeaderSize != 0)
@@ -116,15 +117,12 @@ message_ptr RtpPacketizer::packetize(const binary &payload, bool mark) {
 			                           rtpConfig->rid->length());
 			                           rtpConfig->rid->length());
 		}
 		}
 
 
-		if (rtpConfig->dependencyDescriptorContext.has_value()) {
-			auto &ctx = *rtpConfig->dependencyDescriptorContext;
-			DependencyDescriptorWriter writer(ctx.structure, ctx.activeChains, ctx.descriptor);
-			auto sizeBits = writer.getSizeBits();
-			size_t sizeBytes = (sizeBits + 7) / 8;
-			std::unique_ptr<std::byte[]> buf(new std::byte[sizeBytes]);
-			writer.writeTo(buf.get(), sizeBits);
+		if (ddWriter.has_value()) {
+			auto sizeBytes = ddWriter->getSize();
+			std::vector<std::byte> buf(sizeBytes);
+			ddWriter->writeTo(buf.data(), sizeBytes);
 			offset += extHeader->writeHeader(
 			offset += extHeader->writeHeader(
-			    twoByteHeader, offset, rtpConfig->dependencyDescriptorId, buf.get(), sizeBytes);
+			    twoByteHeader, offset, rtpConfig->dependencyDescriptorId, buf.data(), sizeBytes);
 		}
 		}
 
 
 		if (setPlayoutDelay) {
 		if (setPlayoutDelay) {