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

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

Index: extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc
diff --git a/extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc b/extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc
index 70833fbbc99d2a30713b3d091fe941d8803bcb76..84491386c71361a5606f8b77ea923792c2de8026 100644
--- a/extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc
+++ b/extensions/renderer/api/display_source/wifi_display/wifi_display_video_encoder.cc
@@ -28,6 +28,22 @@ void WiFiDisplayVideoEncoder::Create(
   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) {