From 26fa6b210337aef9689a9fb57f82491734abe8b4 Mon Sep 17 00:00:00 2001 From: charujain Date: Mon, 28 Nov 2016 05:34:07 -0800 Subject: [PATCH] Revert of Bug in ExtractFrame API (extracts frames incorrectly) (patchset #9 id:130001 of https://codereview.webrtc.org/2529923002/ ) Reason for revert: Breaking some trybots due to memory error. Original issue's description: > Fixed bug in ExtractFrameFromY4mFile API which was not extracting the frames correctly. > > Issue: This API was calculating the file_header and frame_header offset only for the first frame which is not the right logic. We need to skip the file and frame header every time we extract a new frame. > > Also added a unit test case which compares the extracted frame with the frame stored in text file. > > BUG=webrtc:6761 > > NOPRESUBMIT=true > NOTRY=true > > Committed: https://crrev.com/b7636b4656d7f8c368963f2256dc2ef7b7ba89c8 > Cr-Commit-Position: refs/heads/master@{#15260} TBR=phoglund@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:6761 Review-Url: https://codereview.webrtc.org/2535783002 Cr-Commit-Position: refs/heads/master@{#15262} --- .../reference_less_video_test_file.y4m.sha1 | 1 - .../video_quality_analysis_frame.txt.sha1 | 1 - webrtc/tools/BUILD.gn | 6 +- .../frame_analyzer/video_quality_analysis.cc | 59 +++++++++---------- .../video_quality_analysis_unittest.cc | 26 -------- 5 files changed, 30 insertions(+), 63 deletions(-) delete mode 100644 resources/reference_less_video_test_file.y4m.sha1 delete mode 100644 resources/video_quality_analysis_frame.txt.sha1 diff --git a/resources/reference_less_video_test_file.y4m.sha1 b/resources/reference_less_video_test_file.y4m.sha1 deleted file mode 100644 index f9e30487dc..0000000000 --- a/resources/reference_less_video_test_file.y4m.sha1 +++ /dev/null @@ -1 +0,0 @@ -5a1620516daf59870158a66230d7bafd9fe9afa1 \ No newline at end of file diff --git a/resources/video_quality_analysis_frame.txt.sha1 b/resources/video_quality_analysis_frame.txt.sha1 deleted file mode 100644 index e4a7c73280..0000000000 --- a/resources/video_quality_analysis_frame.txt.sha1 +++ /dev/null @@ -1 +0,0 @@ -4d1ac894f1743af8059e8d8ae2465f6eaa1790b0 \ No newline at end of file diff --git a/webrtc/tools/BUILD.gn b/webrtc/tools/BUILD.gn index 565a0a2d66..ace0b51c5a 100644 --- a/webrtc/tools/BUILD.gn +++ b/webrtc/tools/BUILD.gn @@ -234,11 +234,7 @@ if (rtc_include_tests) { ] } - tools_unittests_resources = [ - "//resources/foreman_cif.yuv", - "//resources/reference_less_video_test_file.y4m", - "//resources/video_quality_analysis_frame.txt", - ] + tools_unittests_resources = [ "//resources/foreman_cif.yuv" ] if (is_ios) { bundle_data("tools_unittests_bundle_data") { diff --git a/webrtc/tools/frame_analyzer/video_quality_analysis.cc b/webrtc/tools/frame_analyzer/video_quality_analysis.cc index ca78cc0ba7..b28a5d2def 100644 --- a/webrtc/tools/frame_analyzer/video_quality_analysis.cc +++ b/webrtc/tools/frame_analyzer/video_quality_analysis.cc @@ -13,6 +13,7 @@ #include #include #include + #include #define STATS_LINE_LENGTH 32 @@ -125,8 +126,8 @@ bool ExtractFrameFromY4mFile(const char* y4m_file_name, int frame_number, uint8_t* result_frame) { int frame_size = GetI420FrameSize(width, height); - int inital_offset = frame_number * (frame_size + Y4M_FRAME_HEADER_SIZE); - int frame_offset = 0; + int frame_offset = frame_number * frame_size; + bool errors = false; FILE* input_file = fopen(y4m_file_name, "rb"); if (input_file == NULL) { @@ -137,43 +138,41 @@ bool ExtractFrameFromY4mFile(const char* y4m_file_name, // 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". - 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(frame_size) && ferror(input_file)) { - fprintf(stdout, "Error while reading frame from file %s\n", - y4m_file_name); - fclose(input_file); - return false; + // 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(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(found); } - 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(found); // Change stream pointer to new offset, skipping the frame header as well. - fseek(input_file, inital_offset + frame_offset + Y4M_FRAME_HEADER_SIZE, - SEEK_SET); + fseek(input_file, frame_offset + Y4M_FRAME_HEADER_SIZE, SEEK_SET); - bytes_read = fread(result_frame, 1, frame_size, input_file); - if (feof(input_file)) { - fclose(input_file); - return false; - } - if (bytes_read != static_cast(frame_size) && ferror(input_file)) { + size_t bytes_read = fread(result_frame, 1, frame_size, input_file); + if (bytes_read != static_cast(frame_size) && + ferror(input_file)) { fprintf(stdout, "Error while reading frame no %d from file %s\n", frame_number, y4m_file_name); - fclose(input_file); - return false; + errors = true; } fclose(input_file); - return true; + return !errors; } double CalculateMetrics(VideoAnalysisMetricsType video_metrics_type, diff --git a/webrtc/tools/frame_analyzer/video_quality_analysis_unittest.cc b/webrtc/tools/frame_analyzer/video_quality_analysis_unittest.cc index 85c418185f..f8a4b6979f 100644 --- a/webrtc/tools/frame_analyzer/video_quality_analysis_unittest.cc +++ b/webrtc/tools/frame_analyzer/video_quality_analysis_unittest.cc @@ -11,7 +11,6 @@ // This test doesn't actually verify the output since it's just printed // to stdout by void functions, but it's still useful as it executes the code. -#include #include #include @@ -39,31 +38,6 @@ class VideoQualityAnalysisTest : public ::testing::Test { }; FILE* VideoQualityAnalysisTest::logfile_ = NULL; -TEST_F(VideoQualityAnalysisTest, MatchExtractedY4mFrame) { - std::string video_file = - webrtc::test::ResourcePath("reference_less_video_test_file", "y4m"); - - std::string extracted_frame_from_video_file = - webrtc::test::ResourcePath("video_quality_analysis_frame", "txt"); - - int frame_height = 720, frame_width = 1280; - int frame_number = 2; - int size = GetI420FrameSize(frame_width, frame_height); - uint8_t* result_frame = new uint8_t[size]; - uint8_t* expected_frame = new uint8_t[size]; - - FILE* input_file = fopen(extracted_frame_from_video_file.c_str(), "rb"); - fread(expected_frame, 1, size, input_file); - - ExtractFrameFromY4mFile(video_file.c_str(), - frame_width, frame_height, - frame_number, result_frame); - - EXPECT_EQ(*expected_frame, *result_frame); - delete[] result_frame; - delete[] expected_frame; -} - TEST_F(VideoQualityAnalysisTest, PrintAnalysisResultsEmpty) { ResultsContainer result; PrintAnalysisResults(logfile_, "Empty", &result);