From ed425d12c424da2f1652218b3ef04b3547edeac5 Mon Sep 17 00:00:00 2001 From: Emircan Uysaler Date: Thu, 6 Sep 2018 15:35:55 -0700 Subject: [PATCH] Multiplex codec cleanups This CL performs some cleanups on multiplex files: - Adds more comments to factory about usage. - Moves image packer outside /include as it doesn't need to be public. - Other small lint issues. Bug: webrtc:9632 Change-Id: I2e2e6929ea13645aee5483a3697199d1e6184b32 Reviewed-on: https://webrtc-review.googlesource.com/98700 Commit-Queue: Emircan Uysaler Reviewed-by: Niklas Enbom Cr-Commit-Position: refs/heads/master@{#24615} --- media/engine/multiplexcodecfactory.h | 33 ++++++++++++++----- modules/video_coding/BUILD.gn | 2 +- .../include/multiplex_decoder_adapter.h | 2 +- .../include/multiplex_encoder_adapter.h | 4 +-- .../multiplex/multiplex_decoder_adapter.cc | 13 ++++---- .../multiplex_encoded_image_packer.cc | 3 +- .../multiplex_encoded_image_packer.h | 6 ++-- .../test/multiplex_adapter_unittest.cc | 2 +- 8 files changed, 41 insertions(+), 24 deletions(-) rename modules/video_coding/codecs/multiplex/{include => }/multiplex_encoded_image_packer.h (94%) diff --git a/media/engine/multiplexcodecfactory.h b/media/engine/multiplexcodecfactory.h index 030904f1b8..7a851c8c1e 100644 --- a/media/engine/multiplexcodecfactory.h +++ b/media/engine/multiplexcodecfactory.h @@ -18,13 +18,29 @@ #include "api/video_codecs/video_encoder_factory.h" namespace webrtc { - +// Multiplex codec is a completely modular/optional codec that allows users to +// send more than a frame's opaque content(RGB/YUV) over video channels. +// - Allows sending Alpha channel over the wire iff input is +// I420ABufferInterface. Users can expect to receive I420ABufferInterface as the +// decoded video frame buffer. I420A data is split into YUV/AXX portions, +// encoded/decoded seperately and bitstreams are concatanated. +// - Allows sending augmenting data over the wire attached to the frame. This +// attached data portion is not encoded in any way and sent as it is. Users can +// input AugmentedVideoFrameBuffer and can expect the same interface as the +// decoded video frame buffer. +// - Showcases an example of how to add a custom codec in webrtc video channel. +// How to use it end-to-end: +// - Wrap your existing VideoEncoderFactory implemention with +// MultiplexEncoderFactory and VideoDecoderFactory implemention with +// MultiplexDecoderFactory below. For actual coding, multiplex creates encoder +// and decoder instance(s) using these factories. +// - Use Multiplex*coderFactory classes in CreatePeerConnectionFactory() calls. +// - Select "multiplex" codec in SDP negotiation. class MultiplexEncoderFactory : public VideoEncoderFactory { public: - // supports_augmenting_data defines if the encoder would support augmenting - // data in that case the encoder expects video frame buffer of type - // AugmentedVideoFrameBuffer the encoder would encode the attached buffer and - // data together if the flag is not set any frame buffer can be passed in + // |supports_augmenting_data| defines if the encoder would support augmenting + // data. If set, the encoder expects to receive video frame buffers of type + // AugmentedVideoFrameBuffer. MultiplexEncoderFactory(std::unique_ptr factory, bool supports_augmenting_data = false); @@ -40,10 +56,9 @@ class MultiplexEncoderFactory : public VideoEncoderFactory { class MultiplexDecoderFactory : public VideoDecoderFactory { public: - // supports_augmenting_data defines if the decoder would support augmenting - // data in that case the decoder expects the encoded video frame to contain - // augmenting_data it is expected that the sender is using MultiplexEncoder - // with supports_augmenting_data set + // |supports_augmenting_data| defines if the decoder would support augmenting + // data. If set, the decoder is expected to output video frame buffers of type + // AugmentedVideoFrameBuffer. MultiplexDecoderFactory(std::unique_ptr factory, bool supports_augmenting_data = false); diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index 6624069b46..8103d94cca 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -360,10 +360,10 @@ rtc_static_library("webrtc_multiplex") { "codecs/multiplex/augmented_video_frame_buffer.cc", "codecs/multiplex/include/augmented_video_frame_buffer.h", "codecs/multiplex/include/multiplex_decoder_adapter.h", - "codecs/multiplex/include/multiplex_encoded_image_packer.h", "codecs/multiplex/include/multiplex_encoder_adapter.h", "codecs/multiplex/multiplex_decoder_adapter.cc", "codecs/multiplex/multiplex_encoded_image_packer.cc", + "codecs/multiplex/multiplex_encoded_image_packer.h", "codecs/multiplex/multiplex_encoder_adapter.cc", ] diff --git a/modules/video_coding/codecs/multiplex/include/multiplex_decoder_adapter.h b/modules/video_coding/codecs/multiplex/include/multiplex_decoder_adapter.h index b8a90b4e0f..73b67c94d8 100644 --- a/modules/video_coding/codecs/multiplex/include/multiplex_decoder_adapter.h +++ b/modules/video_coding/codecs/multiplex/include/multiplex_decoder_adapter.h @@ -24,7 +24,7 @@ namespace webrtc { class MultiplexDecoderAdapter : public VideoDecoder { public: - // |factory| is not owned and expected to outlive this class' lifetime. + // |factory| is not owned and expected to outlive this class. MultiplexDecoderAdapter(VideoDecoderFactory* factory, const SdpVideoFormat& associated_format, bool supports_augmenting_data = false); diff --git a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h index a24932302f..531d07870e 100644 --- a/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h +++ b/modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h @@ -18,7 +18,7 @@ #include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" -#include "modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h" +#include "modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h" #include "modules/video_coding/include/video_codec_interface.h" #include "rtc_base/criticalsection.h" @@ -32,7 +32,7 @@ enum AlphaCodecStream { class MultiplexEncoderAdapter : public VideoEncoder { public: - // |factory| is not owned and expected to outlive this class' lifetime. + // |factory| is not owned and expected to outlive this class. MultiplexEncoderAdapter(VideoEncoderFactory* factory, const SdpVideoFormat& associated_format, bool supports_augmenting_data = false); diff --git a/modules/video_coding/codecs/multiplex/multiplex_decoder_adapter.cc b/modules/video_coding/codecs/multiplex/multiplex_decoder_adapter.cc index 1a775af954..648e412bba 100644 --- a/modules/video_coding/codecs/multiplex/multiplex_decoder_adapter.cc +++ b/modules/video_coding/codecs/multiplex/multiplex_decoder_adapter.cc @@ -16,6 +16,7 @@ #include "common_video/include/video_frame_buffer.h" #include "common_video/libyuv/include/webrtc_libyuv.h" #include "modules/video_coding/codecs/multiplex/include/augmented_video_frame_buffer.h" +#include "modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h" #include "rtc_base/keep_ref_until_done.h" #include "rtc_base/logging.h" @@ -181,14 +182,14 @@ void MultiplexDecoderAdapter::Decoded(AlphaCodecStream stream_idx, decoded_data_.find(decoded_image->timestamp()); const auto& augmenting_data_it = decoded_augmenting_data_.find(decoded_image->timestamp()); + const bool has_augmenting_data = + augmenting_data_it != decoded_augmenting_data_.end(); if (other_decoded_data_it != decoded_data_.end()) { uint16_t augmenting_data_size = - augmenting_data_it == decoded_augmenting_data_.end() - ? 0 - : augmenting_data_it->second.size_; + has_augmenting_data ? augmenting_data_it->second.size_ : 0; std::unique_ptr augmenting_data = - augmenting_data_size == 0 ? NULL - : std::move(augmenting_data_it->second.data_); + has_augmenting_data ? std::move(augmenting_data_it->second.data_) + : nullptr; auto& other_image_data = other_decoded_data_it->second; if (stream_idx == kYUVStream) { RTC_DCHECK_EQ(kAXXStream, other_image_data.stream_idx_); @@ -205,7 +206,7 @@ void MultiplexDecoderAdapter::Decoded(AlphaCodecStream stream_idx, std::move(augmenting_data), augmenting_data_size); } decoded_data_.erase(decoded_data_.begin(), other_decoded_data_it); - if (supports_augmenting_data_) { + if (has_augmenting_data) { decoded_augmenting_data_.erase(decoded_augmenting_data_.begin(), augmenting_data_it); } diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc index fd316cf694..dec60d92fe 100644 --- a/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc +++ b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.cc @@ -8,9 +8,10 @@ * be found in the AUTHORS file in the root of the source tree. */ -#include "modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h" +#include "modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h" #include +#include #include "modules/rtp_rtcp/source/byte_io.h" diff --git a/modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h similarity index 94% rename from modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h rename to modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h index 220221a849..ea37f0c4d9 100644 --- a/modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h +++ b/modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h @@ -8,8 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ -#ifndef MODULES_VIDEO_CODING_CODECS_MULTIPLEX_INCLUDE_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ -#define MODULES_VIDEO_CODING_CODECS_MULTIPLEX_INCLUDE_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ +#ifndef MODULES_VIDEO_CODING_CODECS_MULTIPLEX_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ +#define MODULES_VIDEO_CODING_CODECS_MULTIPLEX_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ #include #include @@ -116,4 +116,4 @@ class MultiplexEncodedImagePacker { } // namespace webrtc -#endif // MODULES_VIDEO_CODING_CODECS_MULTIPLEX_INCLUDE_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ +#endif // MODULES_VIDEO_CODING_CODECS_MULTIPLEX_MULTIPLEX_ENCODED_IMAGE_PACKER_H_ diff --git a/modules/video_coding/codecs/multiplex/test/multiplex_adapter_unittest.cc b/modules/video_coding/codecs/multiplex/test/multiplex_adapter_unittest.cc index 56de138f08..c479d8add2 100644 --- a/modules/video_coding/codecs/multiplex/test/multiplex_adapter_unittest.cc +++ b/modules/video_coding/codecs/multiplex/test/multiplex_adapter_unittest.cc @@ -17,8 +17,8 @@ #include "media/base/mediaconstants.h" #include "modules/video_coding/codecs/multiplex/include/augmented_video_frame_buffer.h" #include "modules/video_coding/codecs/multiplex/include/multiplex_decoder_adapter.h" -#include "modules/video_coding/codecs/multiplex/include/multiplex_encoded_image_packer.h" #include "modules/video_coding/codecs/multiplex/include/multiplex_encoder_adapter.h" +#include "modules/video_coding/codecs/multiplex/multiplex_encoded_image_packer.h" #include "modules/video_coding/codecs/test/video_codec_unittest.h" #include "modules/video_coding/codecs/vp9/include/vp9.h" #include "rtc_base/keep_ref_until_done.h"