Simplify VideoReader and VideoWriter

remoting/protocol/video_reader.h

 // Copyright (c) 2012 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.
 
-// VideoReader is a generic interface used by ConnectionToHost to read
-// video stream. ProtobufVideoReader implements this interface for
-// protobuf video stream.
-
 #ifndef REMOTING_PROTOCOL_VIDEO_READER_H_
 #define REMOTING_PROTOCOL_VIDEO_READER_H_
 
-#include "base/callback.h"
-#include "base/memory/scoped_ptr.h"
-#include "remoting/protocol/video_stub.h"
+#include "base/compiler_specific.h"
+#include "remoting/proto/video.pb.h"
+#include "remoting/protocol/channel_dispatcher_base.h"
+#include "remoting/protocol/message_reader.h"
+
+namespace net {
+class StreamSocket;
+}  // namespace net
 
 namespace remoting {
 namespace protocol {
 
+class StreamChannelFactory;
 class Session;
-class SessionConfig;
+class VideoStub;
 
-class VideoReader {
+class VideoReader : public ChannelDispatcherBase {
  public:
-  // The callback is called when initialization is finished. The
-  // parameter is set to true on success.
-  typedef base::Callback<void(bool)> InitializedCallback;
-
+  VideoReader();
   virtual ~VideoReader();
 
-  static scoped_ptr<VideoReader> Create(const SessionConfig& config);
-
-  // Initializies the reader. Doesn't take ownership of either |connection|
-  // or |video_stub|.
-  virtual void Init(Session* session,
-                    VideoStub* video_stub,
-                    const InitializedCallback& callback) = 0;
-  virtual bool is_connected() = 0;
+  void set_video_stub(VideoStub* video_stub) { video_stub_ = video_stub; }

[Wez, 2014/09/16 23:10:07]

Why does this need a setter rather than being a constructor parameter?

[Sergey Ulanov, 2014/09/17 21:52:06]

This is consistent with other channel dispatchers.

[Wez, 2014/09/18 00:23:54]

Acknowledged.

 
  protected:
-  VideoReader() { }
+  // ChannelDispatcherBase overrides.
+  virtual void OnInitialized() OVERRIDE;
 
  private:
+  void OnNewData(scoped_ptr<VideoPacket> packet,
+                 const base::Closure& done_task);

[Wez, 2014/09/16 23:10:06]

OnVideoPacket?

[Sergey Ulanov, 2014/09/17 21:52:06]

Done.

+
+  StreamChannelFactory* channel_factory_;

[Wez, 2014/09/16 23:10:06]

This doesn't seem to be used?

[Sergey Ulanov, 2014/09/17 21:52:06]

Done.

+  ProtobufMessageReader<VideoPacket> reader_;

[Wez, 2014/09/16 23:10:06]

It ends up looking a little strange to have VideoReader then contain a class
that does all the actual reading; this outer class is really just a pump between
a StreamSocket and a VideoStub.

[Sergey Ulanov, 2014/09/17 21:52:06]

Renamed this class

+
+  // The stub that processes all received packets.

[Wez, 2014/09/16 23:10:06]

Or "Interface to which VideoPackets are passed for processing."?

[Sergey Ulanov, 2014/09/17 21:52:06]

Done.

+  VideoStub* video_stub_;
+
   DISALLOW_COPY_AND_ASSIGN(VideoReader);
 };
 
 }  // namespace protocol
 }  // namespace remoting
 
 #endif  // REMOTING_PROTOCOL_VIDEO_READER_H_