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 <emircan@webrtc.org>
Reviewed-by: Niklas Enbom <niklas.enbom@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24615}

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
+  // |supports_augmenting_data| defines if the encoder would support augmenting
-  // data in that case the encoder expects video frame buffer of type
+  // data. If set, the encoder expects to receive video frame buffers of type
-  // AugmentedVideoFrameBuffer the encoder would encode the attached buffer and
+  // AugmentedVideoFrameBuffer.
-  // data together if the flag is not set any frame buffer can be passed in
   MultiplexEncoderFactory(std::unique_ptr<VideoEncoderFactory> 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
+  // |supports_augmenting_data| defines if the decoder would support augmenting
-  // data in that case the decoder expects the encoded video frame to contain
+  // data. If set, the decoder is expected to output video frame buffers of type
-  // augmenting_data it is expected that the sender is using MultiplexEncoder
+  // AugmentedVideoFrameBuffer.
-  // with supports_augmenting_data set
   MultiplexDecoderFactory(std::unique_ptr<VideoDecoderFactory> factory,
                           bool supports_augmenting_data = false);
 

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",
   ]
 

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);

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);

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()
+        has_augmenting_data ? augmenting_data_it->second.size_ : 0;
-            ? 0
-            : augmenting_data_it->second.size_;
     std::unique_ptr<uint8_t[]> augmenting_data =
-        augmenting_data_size == 0 ? NULL
+        has_augmenting_data ? std::move(augmenting_data_it->second.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);
     }

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 <cstring>
+#include <utility>
 
 #include "modules/rtp_rtcp/source/byte_io.h"
 

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_
+#ifndef MODULES_VIDEO_CODING_CODECS_MULTIPLEX_MULTIPLEX_ENCODED_IMAGE_PACKER_H_
-#define MODULES_VIDEO_CODING_CODECS_MULTIPLEX_INCLUDE_MULTIPLEX_ENCODED_IMAGE_PACKER_H_
+#define MODULES_VIDEO_CODING_CODECS_MULTIPLEX_MULTIPLEX_ENCODED_IMAGE_PACKER_H_
 
 #include <memory>
 #include <vector>
@@ -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_

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"