Bug in ExtractFrame API (extracts frames incorrectly)

webrtc/tools/frame_analyzer/video_quality_analysis.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/tools/frame_analyzer/video_quality_analysis.h"
 
 #include <assert.h>
 #include <stdio.h>
 #include <stdlib.h>
-
 #include <string>
 
 #define STATS_LINE_LENGTH 32
 #define Y4M_FILE_HEADER_MAX_SIZE 200
 #define Y4M_FRAME_DELIMITER "FRAME"
 #define Y4M_FRAME_HEADER_SIZE 6
 
 namespace webrtc {
 namespace test {
 
@@ skipping 92 matching lines @@
   fclose(input_file);
   return !errors;
 }
 
 bool ExtractFrameFromY4mFile(const char* y4m_file_name,
                              int width,
                              int height,
                              int frame_number,
                              uint8_t* result_frame) {
   int frame_size = GetI420FrameSize(width, height);
-  int frame_offset = frame_number * frame_size;
-  bool errors = false;
+  int inital_offset = frame_number * (frame_size + Y4M_FRAME_HEADER_SIZE);
+  int frame_offset = 0;
 
   FILE* input_file = fopen(y4m_file_name, "rb");
   if (input_file == NULL) {
     fprintf(stderr, "Couldn't open input file for reading: %s\n",
             y4m_file_name);
     return false;
   }
 
   // YUV4MPEG2, a.k.a. Y4M File format has a file header and a frame header. The
   // file header has the aspect: "YUV4MPEG2 C420 W640 H360 Ip F30:1 A1:1".
-  // Skip the header if this is the first frame of the file.
-  if (frame_number == 0) {
-    char frame_header[Y4M_FILE_HEADER_MAX_SIZE];
-    size_t bytes_read =
-        fread(frame_header, 1, Y4M_FILE_HEADER_MAX_SIZE, input_file);
-    if (bytes_read != static_cast<size_t>(frame_size) && ferror(input_file)) {
-      fprintf(stdout, "Error while reading first frame from file %s\n",
-          y4m_file_name);
-      fclose(input_file);
-      return false;
-    }
-    std::string header_contents(frame_header);
-    std::size_t found = header_contents.find(Y4M_FRAME_DELIMITER);
-    if (found == std::string::npos) {
-      fprintf(stdout, "Corrupted Y4M header, could not find \"FRAME\" in %s\n",
-          header_contents.c_str());
-      fclose(input_file);
-      return false;
-    }
-    frame_offset = static_cast<int>(found);
+  char frame_header[Y4M_FILE_HEADER_MAX_SIZE];
+  size_t bytes_read =
+      fread(frame_header, 1, Y4M_FILE_HEADER_MAX_SIZE, input_file);
+  if (bytes_read != static_cast<size_t>(frame_size) && ferror(input_file)) {
+    fprintf(stdout, "Error while reading frame from file %s\n",
+        y4m_file_name);
+    fclose(input_file);
+    return false;
   }
+  std::string header_contents(frame_header);
+  std::size_t found = header_contents.find(Y4M_FRAME_DELIMITER);
+  if (found == std::string::npos) {
+    fprintf(stdout, "Corrupted Y4M header, could not find \"FRAME\" in %s\n",
+        header_contents.c_str());
+    fclose(input_file);
+    return false;
+  }
+  frame_offset = static_cast<int>(found);
 
   // Change stream pointer to new offset, skipping the frame header as well.
-  fseek(input_file, frame_offset + Y4M_FRAME_HEADER_SIZE, SEEK_SET);
+  fseek(input_file, inital_offset + frame_offset + Y4M_FRAME_HEADER_SIZE,
+        SEEK_SET);
 
-  size_t bytes_read = fread(result_frame, 1, frame_size, input_file);
-  if (bytes_read != static_cast<size_t>(frame_size) &&
-      ferror(input_file)) {
+  bytes_read = fread(result_frame, 1, frame_size, input_file);
+  if (feof(input_file)) {

[phoglund, 2016/11/28 12:37:37]

Hmm, you need a fclose here though.

+    return false;
+  }
+  if (bytes_read != static_cast<size_t>(frame_size) && ferror(input_file)) {
     fprintf(stdout, "Error while reading frame no %d from file %s\n",
             frame_number, y4m_file_name);
-    errors = true;
+    fclose(input_file);
+    return false;
   }
 
   fclose(input_file);
-  return !errors;
+  return true;
 }
 
 double CalculateMetrics(VideoAnalysisMetricsType video_metrics_type,
                         const uint8_t* ref_frame,
                         const uint8_t* test_frame,
                         int width,
                         int height) {
   if (!ref_frame || !test_frame)
     return -1;
   else if (height < 0 || width < 0)
@@ skipping 180 matching lines @@
     for (iter = results->frames.begin(); iter != results->frames.end() - 1;
          ++iter) {
       fprintf(output, "%f,", iter->ssim_value);
     }
     fprintf(output, "%f] score\n", iter->ssim_value);
   }
 }
 
 }  // namespace test
 }  // namespace webrtc