I420VideoFrame: Remove functions set_width, set_height, and ResetSize
The functions set_width, set_height, and ResetSize in I420VideoFrame are not needed and just add complexity. R=perkj@webrtc.org, stefan@webrtc.org, tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/39939004 Cr-Commit-Position: refs/heads/master@{#8434} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8434 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
		| @@ -151,33 +151,8 @@ int I420VideoFrame::stride(PlaneType type) const { | |||||||
|   return -1; |   return -1; | ||||||
| } | } | ||||||
|  |  | ||||||
| int I420VideoFrame::set_width(int width) { |  | ||||||
|   if (CheckDimensions(width, height_, |  | ||||||
|                       y_plane_.stride(), u_plane_.stride(), |  | ||||||
|                       v_plane_.stride()) < 0) |  | ||||||
|     return -1; |  | ||||||
|   width_ = width; |  | ||||||
|   return 0; |  | ||||||
| } |  | ||||||
|  |  | ||||||
| int I420VideoFrame::set_height(int height) { |  | ||||||
|   if (CheckDimensions(width_, height, |  | ||||||
|                       y_plane_.stride(), u_plane_.stride(), |  | ||||||
|                       v_plane_.stride()) < 0) |  | ||||||
|     return -1; |  | ||||||
|   height_ = height; |  | ||||||
|   return 0; |  | ||||||
| } |  | ||||||
|  |  | ||||||
| bool I420VideoFrame::IsZeroSize() const { | bool I420VideoFrame::IsZeroSize() const { | ||||||
|   return (y_plane_.IsZeroSize() && u_plane_.IsZeroSize() && |   return width() == 0 || height() == 0; | ||||||
|     v_plane_.IsZeroSize()); |  | ||||||
| } |  | ||||||
|  |  | ||||||
| void I420VideoFrame::ResetSize() { |  | ||||||
|   y_plane_.ResetSize(); |  | ||||||
|   u_plane_.ResetSize(); |  | ||||||
|   v_plane_.ResetSize(); |  | ||||||
| } | } | ||||||
|  |  | ||||||
| void* I420VideoFrame::native_handle() const { return NULL; } | void* I420VideoFrame::native_handle() const { return NULL; } | ||||||
|   | |||||||
| @@ -44,13 +44,8 @@ TEST(TestI420VideoFrame, InitialValues) { | |||||||
| TEST(TestI420VideoFrame, WidthHeightValues) { | TEST(TestI420VideoFrame, WidthHeightValues) { | ||||||
|   I420VideoFrame frame; |   I420VideoFrame frame; | ||||||
|   const int valid_value = 10; |   const int valid_value = 10; | ||||||
|   const int invalid_value = -1; |  | ||||||
|   EXPECT_EQ(0, frame.CreateEmptyFrame(10, 10, 10, 14, 90)); |   EXPECT_EQ(0, frame.CreateEmptyFrame(10, 10, 10, 14, 90)); | ||||||
|   EXPECT_EQ(valid_value, frame.width()); |   EXPECT_EQ(valid_value, frame.width()); | ||||||
|   EXPECT_EQ(invalid_value, frame.set_width(invalid_value)); |  | ||||||
|   EXPECT_EQ(valid_value, frame.height()); |  | ||||||
|   EXPECT_EQ(valid_value, frame.height()); |  | ||||||
|   EXPECT_EQ(invalid_value, frame.set_height(0)); |  | ||||||
|   EXPECT_EQ(valid_value, frame.height()); |   EXPECT_EQ(valid_value, frame.height()); | ||||||
|   frame.set_timestamp(123u); |   frame.set_timestamp(123u); | ||||||
|   EXPECT_EQ(123u, frame.timestamp()); |   EXPECT_EQ(123u, frame.timestamp()); | ||||||
| @@ -76,14 +71,6 @@ TEST(TestI420VideoFrame, SizeAllocation) { | |||||||
|             frame.allocated_size(kVPlane)); |             frame.allocated_size(kVPlane)); | ||||||
| } | } | ||||||
|  |  | ||||||
| TEST(TestI420VideoFrame, ResetSize) { |  | ||||||
|   I420VideoFrame frame; |  | ||||||
|   EXPECT_EQ(0, frame. CreateEmptyFrame(10, 10, 12, 14, 220)); |  | ||||||
|   EXPECT_FALSE(frame.IsZeroSize()); |  | ||||||
|   frame.ResetSize(); |  | ||||||
|   EXPECT_TRUE(frame.IsZeroSize()); |  | ||||||
| } |  | ||||||
|  |  | ||||||
| TEST(TestI420VideoFrame, CopyFrame) { | TEST(TestI420VideoFrame, CopyFrame) { | ||||||
|   uint32_t timestamp = 1; |   uint32_t timestamp = 1; | ||||||
|   int64_t ntp_time_ms = 2; |   int64_t ntp_time_ms = 2; | ||||||
|   | |||||||
| @@ -68,7 +68,6 @@ class TextureVideoFrame : public I420VideoFrame { | |||||||
|   virtual int allocated_size(PlaneType type) const OVERRIDE; |   virtual int allocated_size(PlaneType type) const OVERRIDE; | ||||||
|   virtual int stride(PlaneType type) const OVERRIDE; |   virtual int stride(PlaneType type) const OVERRIDE; | ||||||
|   virtual bool IsZeroSize() const OVERRIDE; |   virtual bool IsZeroSize() const OVERRIDE; | ||||||
|   virtual void ResetSize() OVERRIDE; |  | ||||||
|   virtual void* native_handle() const OVERRIDE; |   virtual void* native_handle() const OVERRIDE; | ||||||
|  |  | ||||||
|  protected: |  protected: | ||||||
|   | |||||||
| @@ -20,8 +20,8 @@ TextureVideoFrame::TextureVideoFrame(NativeHandle* handle, | |||||||
|                                      uint32_t timestamp, |                                      uint32_t timestamp, | ||||||
|                                      int64_t render_time_ms) |                                      int64_t render_time_ms) | ||||||
|     : handle_(handle) { |     : handle_(handle) { | ||||||
|   set_width(width); |   width_ = width; | ||||||
|   set_height(height); |   height_ = height; | ||||||
|   set_timestamp(timestamp); |   set_timestamp(timestamp); | ||||||
|   set_render_time_ms(render_time_ms); |   set_render_time_ms(render_time_ms); | ||||||
| } | } | ||||||
| @@ -107,10 +107,6 @@ bool TextureVideoFrame::IsZeroSize() const { | |||||||
|   return true; |   return true; | ||||||
| } | } | ||||||
|  |  | ||||||
| void TextureVideoFrame::ResetSize() { |  | ||||||
|   assert(false);  // Should not be called. |  | ||||||
| } |  | ||||||
|  |  | ||||||
| void* TextureVideoFrame::native_handle() const { return handle_.get(); } | void* TextureVideoFrame::native_handle() const { return handle_.get(); } | ||||||
|  |  | ||||||
| int TextureVideoFrame::CheckDimensions( | int TextureVideoFrame::CheckDimensions( | ||||||
|   | |||||||
| @@ -40,10 +40,6 @@ TEST(TestTextureVideoFrame, InitialValues) { | |||||||
|   EXPECT_EQ(10, frame.render_time_ms()); |   EXPECT_EQ(10, frame.render_time_ms()); | ||||||
|   EXPECT_EQ(&handle, frame.native_handle()); |   EXPECT_EQ(&handle, frame.native_handle()); | ||||||
|  |  | ||||||
|   EXPECT_EQ(0, frame.set_width(320)); |  | ||||||
|   EXPECT_EQ(320, frame.width()); |  | ||||||
|   EXPECT_EQ(0, frame.set_height(240)); |  | ||||||
|   EXPECT_EQ(240, frame.height()); |  | ||||||
|   frame.set_timestamp(200); |   frame.set_timestamp(200); | ||||||
|   EXPECT_EQ(200u, frame.timestamp()); |   EXPECT_EQ(200u, frame.timestamp()); | ||||||
|   frame.set_render_time_ms(20); |   frame.set_render_time_ms(20); | ||||||
|   | |||||||
| @@ -511,7 +511,6 @@ int32_t VideoFilePlayerImpl::GetVideoFromFile(I420VideoFrame& videoFrame) | |||||||
|     // No new video data read from file. |     // No new video data read from file. | ||||||
|     if(_encodedData.payloadSize == 0) |     if(_encodedData.payloadSize == 0) | ||||||
|     { |     { | ||||||
|         videoFrame.ResetSize(); |  | ||||||
|         return -1; |         return -1; | ||||||
|     } |     } | ||||||
|     int32_t retVal = 0; |     int32_t retVal = 0; | ||||||
|   | |||||||
| @@ -63,7 +63,6 @@ int32_t VideoCoder::SetDecodeCodec(VideoCodec& videoCodecInst, | |||||||
| int32_t VideoCoder::Decode(I420VideoFrame& decodedVideo, | int32_t VideoCoder::Decode(I420VideoFrame& decodedVideo, | ||||||
|                            const EncodedVideoData& encodedData) |                            const EncodedVideoData& encodedData) | ||||||
| { | { | ||||||
|     decodedVideo.ResetSize(); |  | ||||||
|     if(encodedData.payloadSize <= 0) |     if(encodedData.payloadSize <= 0) | ||||||
|     { |     { | ||||||
|         return -1; |         return -1; | ||||||
|   | |||||||
| @@ -93,10 +93,7 @@ int32_t VideoFramesQueue::ReturnFrame(I420VideoFrame* ptrOldFrame) { | |||||||
|   // No need to reuse texture frames because they do not allocate memory. |   // No need to reuse texture frames because they do not allocate memory. | ||||||
|   if (ptrOldFrame->native_handle() == NULL) { |   if (ptrOldFrame->native_handle() == NULL) { | ||||||
|     ptrOldFrame->set_timestamp(0); |     ptrOldFrame->set_timestamp(0); | ||||||
|     ptrOldFrame->set_width(0); |  | ||||||
|     ptrOldFrame->set_height(0); |  | ||||||
|     ptrOldFrame->set_render_time_ms(0); |     ptrOldFrame->set_render_time_ms(0); | ||||||
|     ptrOldFrame->ResetSize(); |  | ||||||
|     _emptyFrames.push_back(ptrOldFrame); |     _emptyFrames.push_back(ptrOldFrame); | ||||||
|   } else { |   } else { | ||||||
|     delete ptrOldFrame; |     delete ptrOldFrame; | ||||||
|   | |||||||
| @@ -71,12 +71,12 @@ MATCHER_P(MatchesVp8StreamInfo, expected, "") { | |||||||
| class EmptyFrameGenerator : public FrameGenerator { | class EmptyFrameGenerator : public FrameGenerator { | ||||||
|  public: |  public: | ||||||
|   virtual I420VideoFrame* NextFrame() OVERRIDE { |   virtual I420VideoFrame* NextFrame() OVERRIDE { | ||||||
|     frame_.ResetSize(); |     frame_.reset(new I420VideoFrame()); | ||||||
|     return &frame_; |     return frame_.get(); | ||||||
|   } |   } | ||||||
|  |  | ||||||
|  private: |  private: | ||||||
|   I420VideoFrame frame_; |   scoped_ptr<I420VideoFrame> frame_; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| class PacketizationCallback : public VCMPacketizationCallback { | class PacketizationCallback : public VCMPacketizationCallback { | ||||||
|   | |||||||
| @@ -91,8 +91,6 @@ TEST_F(VideoProcessingModuleTest, HandleNullBuffer) { | |||||||
|   VideoProcessingModule::FrameStats stats; |   VideoProcessingModule::FrameStats stats; | ||||||
|   // Video frame with unallocated buffer. |   // Video frame with unallocated buffer. | ||||||
|   I420VideoFrame videoFrame; |   I420VideoFrame videoFrame; | ||||||
|   videoFrame.set_width(width_); |  | ||||||
|   videoFrame.set_height(height_); |  | ||||||
|  |  | ||||||
|   EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, videoFrame)); |   EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, videoFrame)); | ||||||
|  |  | ||||||
| @@ -120,21 +118,21 @@ TEST_F(VideoProcessingModuleTest, HandleBadStats) { | |||||||
| TEST_F(VideoProcessingModuleTest, HandleBadSize) { | TEST_F(VideoProcessingModuleTest, HandleBadSize) { | ||||||
|   VideoProcessingModule::FrameStats stats; |   VideoProcessingModule::FrameStats stats; | ||||||
|  |  | ||||||
|   video_frame_.ResetSize(); |   I420VideoFrame bad_frame; | ||||||
|   video_frame_.set_width(width_); |   bad_frame.CreateEmptyFrame(width_, 0, width_, (width_ + 1) / 2, | ||||||
|   video_frame_.set_height(0); |                              (width_ + 1) / 2); | ||||||
|   EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, video_frame_)); |   EXPECT_EQ(-3, vpm_->GetFrameStats(&stats, bad_frame)); | ||||||
|  |  | ||||||
|   EXPECT_EQ(-1, vpm_->ColorEnhancement(&video_frame_)); |   EXPECT_EQ(-1, vpm_->ColorEnhancement(&bad_frame)); | ||||||
|  |  | ||||||
|   EXPECT_EQ(-1, vpm_->Deflickering(&video_frame_, &stats)); |   EXPECT_EQ(-1, vpm_->Deflickering(&bad_frame, &stats)); | ||||||
|  |  | ||||||
|   EXPECT_EQ(-3, vpm_->BrightnessDetection(video_frame_, stats)); |   EXPECT_EQ(-3, vpm_->BrightnessDetection(bad_frame, stats)); | ||||||
|  |  | ||||||
|   EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->SetTargetResolution(0,0,0)); |   EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->SetTargetResolution(0,0,0)); | ||||||
|  |  | ||||||
|   I420VideoFrame *out_frame = NULL; |   I420VideoFrame *out_frame = NULL; | ||||||
|   EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->PreprocessFrame(video_frame_, |   EXPECT_EQ(VPM_PARAMETER_ERROR, vpm_->PreprocessFrame(bad_frame, | ||||||
|                                                        &out_frame)); |                                                        &out_frame)); | ||||||
| } | } | ||||||
|  |  | ||||||
| @@ -335,8 +333,10 @@ void CropFrame(const uint8_t* source_data, | |||||||
|                int cropped_width, |                int cropped_width, | ||||||
|                int cropped_height, |                int cropped_height, | ||||||
|                I420VideoFrame* cropped_frame) { |                I420VideoFrame* cropped_frame) { | ||||||
|   cropped_frame->set_width(cropped_width); |   cropped_frame->CreateEmptyFrame(cropped_width, cropped_height, | ||||||
|   cropped_frame->set_height(cropped_height); |                                   cropped_width, | ||||||
|  |                                   (cropped_width + 1) / 2, | ||||||
|  |                                   (cropped_width + 1) / 2); | ||||||
|   EXPECT_EQ(0, |   EXPECT_EQ(0, | ||||||
|             ConvertToI420(kI420, source_data, offset_x, offset_y, source_width, |             ConvertToI420(kI420, source_data, offset_x, offset_y, source_width, | ||||||
|                           source_height, 0, kRotateNone, cropped_frame)); |                           source_height, 0, kRotateNone, cropped_frame)); | ||||||
|   | |||||||
| @@ -125,7 +125,6 @@ I420VideoFrame* VideoRenderFrames::FrameToRender() { | |||||||
| int32_t VideoRenderFrames::ReturnFrame(I420VideoFrame* old_frame) { | int32_t VideoRenderFrames::ReturnFrame(I420VideoFrame* old_frame) { | ||||||
|   // No need to reuse texture frames because they do not allocate memory. |   // No need to reuse texture frames because they do not allocate memory. | ||||||
|   if (old_frame->native_handle() == NULL) { |   if (old_frame->native_handle() == NULL) { | ||||||
|     old_frame->ResetSize(); |  | ||||||
|     old_frame->set_timestamp(0); |     old_frame->set_timestamp(0); | ||||||
|     old_frame->set_render_time_ms(0); |     old_frame->set_render_time_ms(0); | ||||||
|     empty_frames_.push_back(old_frame); |     empty_frames_.push_back(old_frame); | ||||||
|   | |||||||
| @@ -599,7 +599,6 @@ bool ViECapturer::SwapCapturedAndDeliverFrameIfAvailable() { | |||||||
|   if (deliver_frame_ == NULL) |   if (deliver_frame_ == NULL) | ||||||
|     deliver_frame_.reset(new I420VideoFrame()); |     deliver_frame_.reset(new I420VideoFrame()); | ||||||
|   deliver_frame_->SwapFrame(captured_frame_.get()); |   deliver_frame_->SwapFrame(captured_frame_.get()); | ||||||
|   captured_frame_->ResetSize(); |  | ||||||
|   return true; |   return true; | ||||||
| } | } | ||||||
|  |  | ||||||
|   | |||||||
| @@ -111,12 +111,6 @@ class I420VideoFrame { | |||||||
|   // Get allocated stride per plane. |   // Get allocated stride per plane. | ||||||
|   virtual int stride(PlaneType type) const; |   virtual int stride(PlaneType type) const; | ||||||
|  |  | ||||||
|   // Set frame width. |  | ||||||
|   virtual int set_width(int width); |  | ||||||
|  |  | ||||||
|   // Set frame height. |  | ||||||
|   virtual int set_height(int height); |  | ||||||
|  |  | ||||||
|   // Get frame width. |   // Get frame width. | ||||||
|   virtual int width() const { return width_; } |   virtual int width() const { return width_; } | ||||||
|  |  | ||||||
| @@ -163,10 +157,6 @@ class I420VideoFrame { | |||||||
|   // Return true if underlying plane buffers are of zero size, false if not. |   // Return true if underlying plane buffers are of zero size, false if not. | ||||||
|   virtual bool IsZeroSize() const; |   virtual bool IsZeroSize() const; | ||||||
|  |  | ||||||
|   // Reset underlying plane buffers sizes to 0. This function doesn't |  | ||||||
|   // clear memory. |  | ||||||
|   virtual void ResetSize(); |  | ||||||
|  |  | ||||||
|   // Return the handle of the underlying video frame. This is used when the |   // Return the handle of the underlying video frame. This is used when the | ||||||
|   // frame is backed by a texture. The object should be destroyed when it is no |   // frame is backed by a texture. The object should be destroyed when it is no | ||||||
|   // longer in use, so the underlying resource can be freed. |   // longer in use, so the underlying resource can be freed. | ||||||
| @@ -180,6 +170,8 @@ class I420VideoFrame { | |||||||
|                               int stride_y, |                               int stride_y, | ||||||
|                               int stride_u, |                               int stride_u, | ||||||
|                               int stride_v); |                               int stride_v); | ||||||
|  |   int width_; | ||||||
|  |   int height_; | ||||||
|  |  | ||||||
|  private: |  private: | ||||||
|   // Get the pointer to a specific plane. |   // Get the pointer to a specific plane. | ||||||
| @@ -190,8 +182,6 @@ class I420VideoFrame { | |||||||
|   Plane y_plane_; |   Plane y_plane_; | ||||||
|   Plane u_plane_; |   Plane u_plane_; | ||||||
|   Plane v_plane_; |   Plane v_plane_; | ||||||
|   int width_; |  | ||||||
|   int height_; |  | ||||||
|   uint32_t timestamp_; |   uint32_t timestamp_; | ||||||
|   int64_t ntp_time_ms_; |   int64_t ntp_time_ms_; | ||||||
|   int64_t render_time_ms_; |   int64_t render_time_ms_; | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 magjed@webrtc.org
					magjed@webrtc.org