Implement video frame acknowledgements in the chromoting protocol.

remoting/protocol/client_video_dispatcher.cc

 // Copyright 2014 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 "remoting/protocol/client_video_dispatcher.h"
 
 #include "base/bind.h"
 #include "net/socket/stream_socket.h"
 #include "remoting/base/constants.h"
 #include "remoting/proto/video.pb.h"
+#include "remoting/protocol/errors.h"
+#include "remoting/protocol/message_serialization.h"
 #include "remoting/protocol/video_stub.h"
 
 namespace remoting {
 namespace protocol {
 
 ClientVideoDispatcher::ClientVideoDispatcher(VideoStub* video_stub)
     : ChannelDispatcherBase(kVideoChannelName),
-      parser_(base::Bind(&VideoStub::ProcessVideoPacket,
-                         base::Unretained(video_stub)),
+      video_stub_(video_stub),
+      parser_(base::Bind(&ClientVideoDispatcher::OnVideoPacket,
+                         base::Unretained(this)),
               reader()) {
 }
 
 ClientVideoDispatcher::~ClientVideoDispatcher() {
 }
 
+void ClientVideoDispatcher::OnVideoPacket(scoped_ptr<VideoPacket> video_packet,
+                                          const base::Closure& done) {
+  base::TimeTicks now = base::TimeTicks::Now();
+
+  VideoAck ack_message;

[Wez, 2015/01/21 01:35:38]

Is creating and copying a protobuf super cheap? Consider just passing the two
parameters directly to OnPacketProcessed and creating the ACK message there?

[Sergey Ulanov, 2015/01/29 01:33:28]

Done.

+  ack_message.set_frame_id(video_packet->frame_id());
+
+  ack_message.set_time_to_receive_us(
+      reader()->last_message_read_delay().InMicroseconds());

[Wez, 2015/01/21 01:35:39]

nit: This line is logically part of initializing ack_message, so move it up to
make a since create-and-initialize-ack_message block?

[Sergey Ulanov, 2015/01/29 01:33:28]

Done.

+
+  video_stub_->ProcessVideoPacket(
+      video_packet.Pass(),
+      base::Bind(&ClientVideoDispatcher::OnPacketProcessed,
+                 base::Unretained(this), ack_message, now, done));
+}
+
+void ClientVideoDispatcher::OnPacketProcessed(VideoAck ack_message,
+                                              base::TimeTicks received_time,
+                                              const base::Closure& done) {
+  done.Run();
+
+  // Send Ack message for newer version of the protocol.
+  if (channel_config().version >= kVideoWithAckStreamVersion) {

[Wez, 2015/01/21 01:35:39]

The stream version never changes mid-session, so I'd suggest rearranging things
to check before the Bind(), except that this will become the "normal" path in
future.

So, given that the normal path is to send the ack, I'd suggest rewriting this
if() to be an early-exit.

[Sergey Ulanov, 2015/01/29 01:33:28]

Done.

+    base::TimeDelta rendering_delay = base::TimeTicks::Now() - received_time;
+    ack_message.set_rendering_delay_us(rendering_delay.InMicroseconds());
+    writer()->Write(SerializeAndFrameMessage(ack_message), base::Closure());
+  }
+}
+
 }  // namespace protocol
 }  // namespace remoting