I420VideoFrame: Remove function ResetSize
This is a partial reland of https://webrtc-codereview.appspot.com/39939004/. The original CL was reverted because ViECapturer use ResetSize/IsZeroSize on |captured_frame_| as a check to make sure each captured frame is only delivered once. Removing ResetSize introduced a race condition where a captured frame could be delivered multiple times. I have fixed this problem in this CL by replacing ResetSize with scoped_ptr::release. BUG=4352 R=perkj@webrtc.org, tommi@webrtc.org Review URL: https://webrtc-codereview.appspot.com/39359004 Cr-Commit-Position: refs/heads/master@{#8561} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8561 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
parent
43f4a47c28
commit
97ed2a4b70
@ -156,12 +156,6 @@ bool I420VideoFrame::IsZeroSize() const {
|
||||
v_plane_.IsZeroSize());
|
||||
}
|
||||
|
||||
void I420VideoFrame::ResetSize() {
|
||||
y_plane_.ResetSize();
|
||||
u_plane_.ResetSize();
|
||||
v_plane_.ResetSize();
|
||||
}
|
||||
|
||||
void* I420VideoFrame::native_handle() const { return NULL; }
|
||||
|
||||
int I420VideoFrame::CheckDimensions(int width, int height,
|
||||
|
@ -71,14 +71,6 @@ TEST(TestI420VideoFrame, SizeAllocation) {
|
||||
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) {
|
||||
uint32_t timestamp = 1;
|
||||
int64_t ntp_time_ms = 2;
|
||||
|
@ -68,7 +68,6 @@ class TextureVideoFrame : public I420VideoFrame {
|
||||
virtual int allocated_size(PlaneType type) const OVERRIDE;
|
||||
virtual int stride(PlaneType type) const OVERRIDE;
|
||||
virtual bool IsZeroSize() const OVERRIDE;
|
||||
virtual void ResetSize() OVERRIDE;
|
||||
virtual void* native_handle() const OVERRIDE;
|
||||
|
||||
protected:
|
||||
|
@ -107,10 +107,6 @@ bool TextureVideoFrame::IsZeroSize() const {
|
||||
return true;
|
||||
}
|
||||
|
||||
void TextureVideoFrame::ResetSize() {
|
||||
assert(false); // Should not be called.
|
||||
}
|
||||
|
||||
void* TextureVideoFrame::native_handle() const { return handle_.get(); }
|
||||
|
||||
int TextureVideoFrame::CheckDimensions(
|
||||
|
@ -71,12 +71,12 @@ MATCHER_P(MatchesVp8StreamInfo, expected, "") {
|
||||
class EmptyFrameGenerator : public FrameGenerator {
|
||||
public:
|
||||
virtual I420VideoFrame* NextFrame() OVERRIDE {
|
||||
frame_.ResetSize();
|
||||
return &frame_;
|
||||
frame_.reset(new I420VideoFrame());
|
||||
return frame_.get();
|
||||
}
|
||||
|
||||
private:
|
||||
I420VideoFrame frame_;
|
||||
rtc::scoped_ptr<I420VideoFrame> frame_;
|
||||
};
|
||||
|
||||
class PacketizationCallback : public VCMPacketizationCallback {
|
||||
|
@ -125,7 +125,6 @@ I420VideoFrame* VideoRenderFrames::FrameToRender() {
|
||||
int32_t VideoRenderFrames::ReturnFrame(I420VideoFrame* old_frame) {
|
||||
// No need to reuse texture frames because they do not allocate memory.
|
||||
if (old_frame->native_handle() == NULL) {
|
||||
old_frame->ResetSize();
|
||||
old_frame->set_timestamp(0);
|
||||
old_frame->set_render_time_ms(0);
|
||||
empty_frames_.push_back(old_frame);
|
||||
|
@ -610,18 +610,7 @@ bool ViECapturer::SwapCapturedAndDeliverFrameIfAvailable() {
|
||||
if (captured_frame_ == NULL)
|
||||
return false;
|
||||
|
||||
if (captured_frame_->native_handle() != NULL) {
|
||||
deliver_frame_.reset(captured_frame_.release());
|
||||
return true;
|
||||
}
|
||||
|
||||
if (captured_frame_->IsZeroSize())
|
||||
return false;
|
||||
|
||||
if (deliver_frame_ == NULL)
|
||||
deliver_frame_.reset(new I420VideoFrame());
|
||||
deliver_frame_->SwapFrame(captured_frame_.get());
|
||||
captured_frame_->ResetSize();
|
||||
deliver_frame_.reset(captured_frame_.release());
|
||||
return true;
|
||||
}
|
||||
|
||||
|
@ -161,10 +161,6 @@ TEST_F(ViECapturerTest, TestI420Frames) {
|
||||
// Make sure the buffer is swapped and not copied.
|
||||
for (int i = 0; i < kNumFrame; ++i)
|
||||
EXPECT_EQ(ybuffer_pointers[i], output_frame_ybuffers_[i]);
|
||||
// The pipeline should be filled with frames with allocated buffers. Check
|
||||
// the last input frame has the same allocated size after swapping.
|
||||
EXPECT_EQ(input_frames_.back()->allocated_size(kYPlane),
|
||||
copied_input_frames.back()->allocated_size(kYPlane));
|
||||
}
|
||||
|
||||
TEST_F(ViECapturerTest, TestI420FrameAfterTextureFrame) {
|
||||
|
@ -157,10 +157,6 @@ class I420VideoFrame {
|
||||
// Return true if underlying plane buffers are of zero size, false if not.
|
||||
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
|
||||
// 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.
|
||||
|
Loading…
Reference in New Issue
Block a user