ImageDecodeBench packeted runs should resume decoding where it left off

third_party/WebKit/Source/platform/testing/ImageDecodeBench.cpp

 // Copyright 2015 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.
 
 // Provides a minimal wrapping of the Blink image decoders. Used to perform
 // a non-threaded, memory-to-memory image decode using micro second accuracy
 // clocks to measure image decode time. Optionally applies color correction
 // during image decoding on supported platforms (default off). Usage:
 //
 //   % ninja -C out/Release image_decode_bench &&
@@ skipping 208 matching lines @@
     for (int i = 0; i < frame_count; ++i) {
       if (!decoder->FrameBufferAtIndex(i))
         return false;
     }
 
     return !decoder->Failed();
   }
 
   RefPtr<SharedBuffer> packet_data = SharedBuffer::Create();
   size_t position = 0;
+  size_t next_frame_to_decode = 0;
   while (true) {
     const char* packet;
     size_t length = data->GetSomeData(packet, position);
 
     length = std::min(length, packet_size);
     packet_data->Append(packet, length);
     position += length;
 
     bool all_data_received = position == data->size();
-    decoder->SetData(packet_data.Get(), all_data_received);
-
-    int frame_count = decoder->FrameCount();
-    for (int i = 0; i < frame_count; ++i) {
-      if (!decoder->FrameBufferAtIndex(i))
+    size_t frame_count = decoder->FrameCount();
+    for (; next_frame_to_decode < frame_count; ++next_frame_to_decode) {
+      decoder->SetData(packet_data.Get(), all_data_received);
+      ImageFrame* frame = decoder->FrameBufferAtIndex(next_frame_to_decode);
+      if (frame->GetStatus() != ImageFrame::kFrameComplete)

[scroggo_chromium, 2017/05/15 12:48:28]

I think this belongs at the end of the loop. Notice how
ImageFrameGenerator::Decode calls SetData(null) and ClearCacheExceptFrame
regardless of the FrameStatus.

[cblume, 2017/05/15 20:55:09]

This is effectively done now that the SetData reset/clear and
ClearCacheExceptFrame have been removed.

         break;
+      decoder->SetData(PassRefPtr<SegmentReader>(nullptr), false);
+      decoder->ClearCacheExceptFrame(next_frame_to_decode);

[scroggo_chromium, 2017/05/15 12:48:28]

I'm ambivalent about adding ClearCacheExceptFrame and removing/resetting the
data. It is more in line with how we do things right now, although if we change
anything it seems like it would be easy to get the two out of sync.

At the very least, it deserves a comment, explaining that this replicates the
behavior in ImageFrameGenerator::Decode, and probably a comment at the top where
it explains what this benchmark does.

[cblume, 2017/05/15 17:39:11]

I agree with you. This runs the risk of becoming out-of-sync. A better option is
an integration test into Chromium, which we already have.

Maybe the real solution here is to just not have a packeted decode test?

I can't think of any reason to have it other than replicating that integration
test.

[scroggo_chromium, 2017/05/15 18:09:22]

Where is the integration test?

FWIW, I added the packet mode because I was working on changes that I expected
might have an impact on decoding in packets that was different than having all
of the data available up front. If that is covered by another test, then perhaps
you're right.

     }
 
     if (all_data_received || decoder->Failed())
       break;
   }
 
   return !decoder->Failed();
 }
 
 }  // namespace
@@ skipping 93 matching lines @@
   double average_time = total_time / static_cast<double>(iterations);
   printf("%f %f\n", total_time, average_time);
   return 0;
 }
 
 }  // namespace blink
 
 int main(int argc, char* argv[]) {
   return blink::Main(argc, argv);
 }