From b8ee20df1de6732ff636f37a83c426fc17754a3c Mon Sep 17 00:00:00 2001 From: Tomohiro IKEDA Date: Wed, 11 Sep 2019 05:15:17 +0900 Subject: [PATCH] Improve ConvertToADTS function performance (#639) Remove the extra data copying. --- .../media/codecs/aac_audio_specific_config.cc | 32 +++++++++++-------- .../media/codecs/aac_audio_specific_config.h | 11 ++++--- .../formats/mp2t/pes_packet_generator.cc | 20 ++++++------ .../mp2t/pes_packet_generator_unittest.cc | 11 ++++--- .../packed_audio/packed_audio_segmenter.cc | 6 ++-- .../packed_audio_segmenter_unittest.cc | 19 +++++------ 6 files changed, 56 insertions(+), 43 deletions(-) diff --git a/packager/media/codecs/aac_audio_specific_config.cc b/packager/media/codecs/aac_audio_specific_config.cc index 572778f84a..486417868a 100644 --- a/packager/media/codecs/aac_audio_specific_config.cc +++ b/packager/media/codecs/aac_audio_specific_config.cc @@ -137,27 +137,33 @@ bool AACAudioSpecificConfig::Parse(const std::vector& data) { channel_config_ <= 7; } -bool AACAudioSpecificConfig::ConvertToADTS(std::vector* buffer) const { - size_t size = buffer->size() + kADTSHeaderSize; - +bool AACAudioSpecificConfig::ConvertToADTS( + const uint8_t* data, + size_t data_size, + std::vector* audio_frame) const { DCHECK(audio_object_type_ >= 1 && audio_object_type_ <= 4 && frequency_index_ != 0xf && channel_config_ <= 7); + size_t size = kADTSHeaderSize + data_size; + // ADTS header uses 13 bits for packet size. if (size >= (1 << 13)) return false; - std::vector& adts = *buffer; + audio_frame->reserve(size); + audio_frame->resize(kADTSHeaderSize); - adts.insert(buffer->begin(), kADTSHeaderSize, 0); - adts[0] = 0xff; - adts[1] = 0xf1; - adts[2] = ((audio_object_type_ - 1) << 6) + (frequency_index_ << 2) + - (channel_config_ >> 2); - adts[3] = ((channel_config_ & 0x3) << 6) + static_cast(size >> 11); - adts[4] = static_cast((size & 0x7ff) >> 3); - adts[5] = static_cast(((size & 7) << 5) + 0x1f); - adts[6] = 0xfc; + audio_frame->at(0) = 0xff; + audio_frame->at(1) = 0xf1; + audio_frame->at(2) = ((audio_object_type_ - 1) << 6) + + (frequency_index_ << 2) + (channel_config_ >> 2); + audio_frame->at(3) = + ((channel_config_ & 0x3) << 6) + static_cast(size >> 11); + audio_frame->at(4) = static_cast((size & 0x7ff) >> 3); + audio_frame->at(5) = static_cast(((size & 7) << 5) + 0x1f); + audio_frame->at(6) = 0xfc; + + audio_frame->insert(audio_frame->end(), data, data + data_size); return true; } diff --git a/packager/media/codecs/aac_audio_specific_config.h b/packager/media/codecs/aac_audio_specific_config.h index 6bd71c44c4..e83199c66f 100644 --- a/packager/media/codecs/aac_audio_specific_config.h +++ b/packager/media/codecs/aac_audio_specific_config.h @@ -84,11 +84,14 @@ class AACAudioSpecificConfig { virtual bool Parse(const std::vector& data); /// Convert a raw AAC frame into an AAC frame with an ADTS header. - /// @param[in,out] buffer contains the raw AAC frame on input, and the - /// converted frame on output if successful; it is untouched - /// on failure. + /// @param data points to the raw AAC frame to be converted. + /// @param data_size the size of a raw AAC frame. + /// @param[out] audio_frame contains the converted frame if successful; it is + /// untouched on failure. /// @return true on success, false otherwise. - virtual bool ConvertToADTS(std::vector* buffer) const; + virtual bool ConvertToADTS(const uint8_t* data, + size_t data_size, + std::vector* audio_frame) const; /// @return The audio object type for this AAC config, with possible extension /// considered. diff --git a/packager/media/formats/mp2t/pes_packet_generator.cc b/packager/media/formats/mp2t/pes_packet_generator.cc index 30a41f9ff2..68b09592ea 100644 --- a/packager/media/formats/mp2t/pes_packet_generator.cc +++ b/packager/media/formats/mp2t/pes_packet_generator.cc @@ -52,7 +52,8 @@ bool PesPacketGenerator::Initialize(const StreamInfo& stream_info) { converter_.reset(new NalUnitToByteStreamConverter()); return converter_->Initialize(video_stream_info.codec_config().data(), video_stream_info.codec_config().size()); - } else if (stream_type_ == kStreamAudio) { + } + if (stream_type_ == kStreamAudio) { const AudioStreamInfo& audio_stream_info = static_cast(stream_info); timescale_scale_ = kTsTimescale / audio_stream_info.time_scale(); @@ -60,8 +61,9 @@ bool PesPacketGenerator::Initialize(const StreamInfo& stream_info) { audio_stream_id_ = kAacAudioStreamId; adts_converter_.reset(new AACAudioSpecificConfig()); return adts_converter_->Parse(audio_stream_info.codec_config()); - } else if (audio_stream_info.codec() == Codec::kCodecAC3 || - audio_stream_info.codec() == Codec::kCodecEAC3) { + } + if (audio_stream_info.codec() == Codec::kCodecAC3 || + audio_stream_info.codec() == Codec::kCodecEAC3) { audio_stream_id_ = kAc3AudioStreamId; // No converter needed for AC3 and E-AC3. return true; @@ -117,17 +119,15 @@ bool PesPacketGenerator::PushSample(const MediaSample& sample) { } DCHECK_EQ(stream_type_, kStreamAudio); - std::vector audio_frame(sample.data(), - sample.data() + sample.data_size()); + std::vector audio_frame; // AAC is carried in ADTS. if (adts_converter_) { - // TODO(rkuroiwa): ConvertToADTS() makes another copy of audio_frame - // internally. Optimize copying in this function, possibly by adding a - // method on AACAudioSpecificConfig that takes {pointer, length} pair and - // returns a vector that has the ADTS header. - if (!adts_converter_->ConvertToADTS(&audio_frame)) + if (!adts_converter_->ConvertToADTS(sample.data(), sample.data_size(), + &audio_frame)) return false; + } else { + audio_frame.assign(sample.data(), sample.data() + sample.data_size()); } // TODO(rkuriowa): Put multiple samples in the PES packet to reduce # of PES diff --git a/packager/media/formats/mp2t/pes_packet_generator_unittest.cc b/packager/media/formats/mp2t/pes_packet_generator_unittest.cc index 40265c8b96..d0f3bf30dc 100644 --- a/packager/media/formats/mp2t/pes_packet_generator_unittest.cc +++ b/packager/media/formats/mp2t/pes_packet_generator_unittest.cc @@ -112,7 +112,10 @@ class MockNalUnitToByteStreamConverter : public NalUnitToByteStreamConverter { class MockAACAudioSpecificConfig : public AACAudioSpecificConfig { public: MOCK_METHOD1(Parse, bool(const std::vector& data)); - MOCK_CONST_METHOD1(ConvertToADTS, bool(std::vector* buffer)); + MOCK_CONST_METHOD3(ConvertToADTS, + bool(const uint8_t* data, + size_t data_size, + std::vector* audio_frame)); }; std::shared_ptr CreateVideoStreamInfo(Codec codec) { @@ -307,8 +310,8 @@ TEST_F(PesPacketGeneratorTest, AddAudioSample) { std::unique_ptr mock( new MockAACAudioSpecificConfig()); - EXPECT_CALL(*mock, ConvertToADTS(_)) - .WillOnce(DoAll(SetArgPointee<0>(expected_data), Return(true))); + EXPECT_CALL(*mock, ConvertToADTS(sample->data(), sample->data_size(), _)) + .WillOnce(DoAll(SetArgPointee<2>(expected_data), Return(true))); UseMockAACAudioSpecificConfig(std::move(mock)); @@ -335,7 +338,7 @@ TEST_F(PesPacketGeneratorTest, AddAudioSampleFailedToConvert) { std::unique_ptr mock( new MockAACAudioSpecificConfig()); - EXPECT_CALL(*mock, ConvertToADTS(_)).WillOnce(Return(false)); + EXPECT_CALL(*mock, ConvertToADTS(_, _, _)).WillOnce(Return(false)); UseMockAACAudioSpecificConfig(std::move(mock)); diff --git a/packager/media/formats/packed_audio/packed_audio_segmenter.cc b/packager/media/formats/packed_audio/packed_audio_segmenter.cc index 1057e8273b..d6628dcd61 100644 --- a/packager/media/formats/packed_audio/packed_audio_segmenter.cc +++ b/packager/media/formats/packed_audio/packed_audio_segmenter.cc @@ -67,9 +67,9 @@ Status PackedAudioSegmenter::AddSample(const MediaSample& sample) { } if (adts_converter_) { - std::vector audio_frame(sample.data(), - sample.data() + sample.data_size()); - if (!adts_converter_->ConvertToADTS(&audio_frame)) + std::vector audio_frame; + if (!adts_converter_->ConvertToADTS(sample.data(), sample.data_size(), + &audio_frame)) return Status(error::MUXER_FAILURE, "Failed to convert to ADTS."); segment_buffer_.AppendArray(audio_frame.data(), audio_frame.size()); } else { diff --git a/packager/media/formats/packed_audio/packed_audio_segmenter_unittest.cc b/packager/media/formats/packed_audio/packed_audio_segmenter_unittest.cc index 3566d5c502..ae03d79ac8 100644 --- a/packager/media/formats/packed_audio/packed_audio_segmenter_unittest.cc +++ b/packager/media/formats/packed_audio/packed_audio_segmenter_unittest.cc @@ -19,9 +19,7 @@ using ::testing::_; using ::testing::ByMove; using ::testing::DoAll; using ::testing::ElementsAreArray; -using ::testing::Eq; using ::testing::Invoke; -using ::testing::Pointee; using ::testing::Return; using ::testing::SetArgPointee; using ::testing::Test; @@ -107,7 +105,10 @@ std::shared_ptr CreateEncryptedSample( class MockAACAudioSpecificConfig : public AACAudioSpecificConfig { public: MOCK_METHOD1(Parse, bool(const std::vector& data)); - MOCK_CONST_METHOD1(ConvertToADTS, bool(std::vector* buffer)); + MOCK_CONST_METHOD3(ConvertToADTS, + bool(const uint8_t* data, + size_t data_size, + std::vector* audio_frame)); }; class MockId3Tag : public Id3Tag { @@ -170,8 +171,8 @@ TEST_F(PackedAudioSegmenterTest, AacAddSample) { EXPECT_CALL(*mock_adts_converter_, Parse(ElementsAreArray(kCodecConfig))) .WillOnce(Return(true)); EXPECT_CALL(*mock_adts_converter_, - ConvertToADTS(Pointee(Eq(StringToVector(kSample1Data))))) - .WillOnce(DoAll(SetArgPointee<0>(StringToVector(kAdtsSample1Data)), + ConvertToADTS(_, sizeof(kSample1Data) - 1, _)) + .WillOnce(DoAll(SetArgPointee<2>(StringToVector(kAdtsSample1Data)), Return(true))); EXPECT_CALL(segmenter_, CreateAdtsConverter()) @@ -199,8 +200,8 @@ TEST_F(PackedAudioSegmenterTest, TruncateLargeTimestamp) { EXPECT_CALL(*mock_adts_converter_, Parse(ElementsAreArray(kCodecConfig))) .WillOnce(Return(true)); EXPECT_CALL(*mock_adts_converter_, - ConvertToADTS(Pointee(Eq(StringToVector(kSample1Data))))) - .WillOnce(DoAll(SetArgPointee<0>(StringToVector(kAdtsSample1Data)), + ConvertToADTS(_, sizeof(kSample1Data) - 1, _)) + .WillOnce(DoAll(SetArgPointee<2>(StringToVector(kAdtsSample1Data)), Return(true))); EXPECT_CALL(segmenter_, CreateAdtsConverter()) @@ -301,8 +302,8 @@ TEST_F(PackedAudioSegmenterTest, AacAddEncryptedSample) { EXPECT_CALL(*mock_adts_converter_, Parse(ElementsAreArray(kCodecConfig))) .WillOnce(Return(true)); EXPECT_CALL(*mock_adts_converter_, - ConvertToADTS(Pointee(Eq(StringToVector(kSample1Data))))) - .WillOnce(DoAll(SetArgPointee<0>(StringToVector(kAdtsSample1Data)), + ConvertToADTS(_, sizeof(kSample1Data) - 1, _)) + .WillOnce(DoAll(SetArgPointee<2>(StringToVector(kAdtsSample1Data)), Return(true))); EXPECT_CALL(segmenter_, CreateAdtsConverter())