[chrome.displaySource] Move video stream info creation to encoders.

extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.h"
 
 #include "base/logging.h"
 
 namespace extensions {
 
@@ skipping 10 matching lines @@
 
 WiFiDisplayVideoEncoder::~WiFiDisplayVideoEncoder() = default;
 
 // static
 void WiFiDisplayVideoEncoder::Create(
     const InitParameters& params,
     const VideoEncoderCallback& encoder_callback) {
   CreateVEA(params, encoder_callback);
 }
 
+WiFiDisplayElementaryStreamInfo
+WiFiDisplayVideoEncoder::CreateElementaryStreamInfo() const {
+  DCHECK(client_thread_checker_.CalledOnValidThread());
+  return WiFiDisplayElementaryStreamInfo(
+      WiFiDisplayElementaryStreamInfo::VIDEO_H264,
+      CreateElementaryStreamDescriptors());
+}
+
+std::vector<WiFiDisplayElementaryStreamDescriptor>
+WiFiDisplayVideoEncoder::CreateElementaryStreamDescriptors() const {

[Mikhail, 2016/04/29 06:36:57]

why this method is needed(isn't CreateElementaryStreamInfo enough)?

[e_hakkinen, 2016/04/29 07:16:13]

The idea is that it can be overridden by WiFiDisplayVideoEncoder{SVC,VEA} so
that SVC and VEA can modify descriptors (particularly insert AVC Video
descriptors) before an info is created.

[Mikhail, 2016/04/29 07:37:41]

CreateElementaryStreamInfo's body is tiny, IMO encoder implementations could
just override it if needed.

[e_hakkinen, 2016/04/29 08:27:07]

With the current design, WiFiDisplayVideoEncoder defines the functions

  WiFiDisplayElementaryStreamInfo CreateElementaryStreamInfo() const final;

  virtual std::vector<WiFiDisplayElementaryStreamDescriptor>
  CreateElementaryStreamDescriptors() const;

and an implementation class would define

  std::vector<WiFiDisplayElementaryStreamDescriptor>
  CreateElementaryStreamDescriptors() const override {
   // Create descriptors common to all H.264 video encoders.
   auto descriptors =
       WiFiDisplayVideoEncoder::CreateElementaryStreamDescriptors();
   // Modify descriptors as needed. At least add an AVCVideo descriptor.
   descriptors.push_back(
       WiFiDisplayElementaryStreamDescriptor::AVCVideo::Create(/* ... */));
   // Return descriptors.
   return descriptors;
  }

With CreateElementaryStreamDescriptors replaced with
CreateCommonElementaryStreamDescriptors, WiFiDisplayVideoEncoder would define
the function

  std::vector<WiFiDisplayElementaryStreamDescriptor>
  CreateCommonElementaryStreamDescriptors() const;

and an implementation class would define

  WiFiDisplayElementaryStreamInfo CreateElementaryStreamInfo() const override {
   // Create descriptors common to all H.264 video encoders.
   auto descriptors = CreateCommonElementaryStreamDescriptors();
   // Modify descriptors as needed. At least add an AVCVideo descriptor.
   descriptors.push_back(
       WiFiDisplayElementaryStreamDescriptor::AVCVideo::Create(/* ... */));
   // Create and return info.
   return WiFiDisplayElementaryStreamInfo(
       WiFiDisplayElementaryStreamInfo::VIDEO_H264, std::move(descriptors));
  }

So in both options, in WiFiDisplayVideoEncoder there is a function for creating
elementary stream descriptors (CreateElementaryStreamDescriptors or
CreateCommonElementaryStreamDescriptors). It does not make sense to drop those
functions completely because they implement common functionality which should
not be reimplemented in each and every implementation class. Although, at the
moment, those functions only create AVC Timing and HRD descriptors they should
eventually create other descriptors such as window info descriptors, too.

Are you saying that you would prefer the latter design more?
In the beginning, you asked whether a function to create descriptors is needed.
It is. But now I am not sure what you are asking. Hopefully this is detailed
enough description for getting us to agreement.

+  std::vector<WiFiDisplayElementaryStreamDescriptor> descriptors;
+  descriptors.push_back(
+      WiFiDisplayElementaryStreamDescriptor::AVCTimingAndHRD::Create());
+  return descriptors;
+}
+
 void WiFiDisplayVideoEncoder::InsertRawVideoFrame(
     const scoped_refptr<media::VideoFrame>& video_frame,
     base::TimeTicks reference_time) {
   DCHECK(client_thread_checker_.CalledOnValidThread());
   DCHECK(!encoded_callback_.is_null());
   media_task_runner_->PostTask(
       FROM_HERE, base::Bind(&WiFiDisplayVideoEncoder::InsertFrameOnMediaThread,
                             this, video_frame, reference_time, send_idr_));
   send_idr_ = false;
 }
 
 void WiFiDisplayVideoEncoder::RequestIDRPicture() {
   DCHECK(client_thread_checker_.CalledOnValidThread());
   send_idr_ = true;
 }
 
 }  // namespace extensions