Commit e19def5a authored by Findit's avatar Findit

Revert "[macOS] PixelBufferPool: Test retaining IOSurface, not CVPixelBuffer."

This reverts commit be3eeb5b.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 824814 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2JlM2VlYjViMzk5NTY5YTZiMWQ4OGY5ZTk1M2M3NmIxNDQxMWJiYTEM

Sample Failed Build: https://ci.chromium.org/b/8864373521456164320

Sample Failed Step: capture_unittests

Original change's description:
> [macOS] PixelBufferPool: Test retaining IOSurface, not CVPixelBuffer.
> 
> Pixel buffers originating from a CVPixelBufferPool are recycled when
> no longer in use, previously only tested by releasing the last pixel
> buffer reference and not touching the IOSurface.
> 
> This CL tests what happens when we are retaining not references to the
> pixel buffer, but references to a pixel buffer's IOSurface or
> incrementing its use count.
> 
> Outcome:
> - Retaining references (base::ScopedCFTypeRef<IOSurfaceRef>) to an
>   IOSurface does NOT prevent recycling.
> - Retaining use count (IOSurfaceIncrementUseCount) to an IOSurface DOES
>   prevent recycling.
> 
> This behavior is now documented in pixel_buffer_pool_mac.h.
> 
> Bug: chromium:1132299
> Change-Id: Ibdcd3752a4467bc495eba4b543ecd73da1b2f41f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2520829
> Commit-Queue: Markus Handell <handellm@google.com>
> Reviewed-by: Markus Handell <handellm@google.com>
> Cr-Commit-Position: refs/heads/master@{#824814}


Change-Id: I9ffde1dcd267cf97ed5918ba86e913189625f3e5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: chromium:1132299
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523054
Cr-Commit-Position: refs/heads/master@{#824891}
parent 242de3b9
...@@ -35,15 +35,6 @@ class CAPTURE_EXPORT PixelBufferPool { ...@@ -35,15 +35,6 @@ class CAPTURE_EXPORT PixelBufferPool {
// Freeing all buffer references returns the underlying buffer to the pool. In // Freeing all buffer references returns the underlying buffer to the pool. In
// order to free memory, you must both release all buffers and call Flush() or // order to free memory, you must both release all buffers and call Flush() or
// delete the pool. It is safe for a buffer to outlive its pool. // delete the pool. It is safe for a buffer to outlive its pool.
//
// Retaining a pixel buffer and preventing it from returning to the pool can
// be done either by keeping a reference directly to the CVPixelBuffer, e.g.
// with a base::ScopedCFTypeRef<CVPixelBufferRef>, or by incrementing the use
// count of the IOSurface, i.e. with IOSurfaceIncrementUseCount().
//
// WARNING: Retaining references to the pixel buffer's IOSurface (e.g. with
// base::ScopedCFTypeRef<IOSurfaceRef>) without incrementing its use count
// does NOT prevent it from being recycled!
base::ScopedCFTypeRef<CVPixelBufferRef> CreateBuffer(); base::ScopedCFTypeRef<CVPixelBufferRef> CreateBuffer();
// Frees the memory of any released buffers returned to the pool. // Frees the memory of any released buffers returned to the pool.
......
...@@ -43,10 +43,9 @@ TEST(PixelBufferPoolTest, CreatedBufferHasIOSurface) { ...@@ -43,10 +43,9 @@ TEST(PixelBufferPoolTest, CreatedBufferHasIOSurface) {
EXPECT_TRUE(CVPixelBufferGetIOSurface(buffer)); EXPECT_TRUE(CVPixelBufferGetIOSurface(buffer));
} }
TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenHoldingOnToPixelBuffer) { TEST(PixelBufferPoolTest, CannotExceedMaxBuffers) {
constexpr size_t kPoolMaxBuffers = 2; std::unique_ptr<PixelBufferPool> pool =
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create( PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 2);
kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer(); base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
EXPECT_TRUE(first_buffer); EXPECT_TRUE(first_buffer);
base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer(); base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
...@@ -55,25 +54,6 @@ TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenHoldingOnToPixelBuffer) { ...@@ -55,25 +54,6 @@ TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenHoldingOnToPixelBuffer) {
EXPECT_FALSE(third_buffer); EXPECT_FALSE(third_buffer);
} }
TEST(PixelBufferPoolTest, CannotExceedMaxBuffersWhenIOSurfaceIsInUse) {
constexpr size_t kPoolMaxBuffers = 1;
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
EXPECT_TRUE(first_buffer);
IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
EXPECT_TRUE(io_surface);
// Incremet use count of raw ptr IOSurface reference while releasing the pixel
// buffer's only reference.
IOSurfaceIncrementUseCount(io_surface);
first_buffer.reset();
// The pixel buffer has not been returned to the pool.
base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
EXPECT_FALSE(second_buffer);
// Cleanup.
IOSurfaceDecrementUseCount(io_surface);
}
TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) { TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create( std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
kPixelFormatNv12, kVgaWidth, kVgaHeight, base::nullopt); kPixelFormatNv12, kVgaWidth, kVgaHeight, base::nullopt);
...@@ -86,58 +66,14 @@ TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) { ...@@ -86,58 +66,14 @@ TEST(PixelBufferPoolTest, CanCreateBuffersIfMaxIsNull) {
} }
TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousBufferIsReleased) { TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousBufferIsReleased) {
constexpr size_t kPoolMaxBuffers = 1; std::unique_ptr<PixelBufferPool> pool =
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create( PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);
kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
base::ScopedCFTypeRef<CVPixelBufferRef> buffer = pool->CreateBuffer(); base::ScopedCFTypeRef<CVPixelBufferRef> buffer = pool->CreateBuffer();
buffer.reset(); buffer.reset();
buffer = pool->CreateBuffer(); buffer = pool->CreateBuffer();
EXPECT_TRUE(buffer); EXPECT_TRUE(buffer);
} }
TEST(PixelBufferPoolTest, CanCreateBufferAfterPreviousIOSurfaceIsNoLongerUsed) {
constexpr size_t kPoolMaxBuffers = 1;
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
EXPECT_TRUE(first_buffer);
IOSurfaceRef io_surface = CVPixelBufferGetIOSurface(first_buffer);
EXPECT_TRUE(io_surface);
IOSurfaceIncrementUseCount(io_surface);
first_buffer.reset();
// Decrementing the use count when there are no pixel buffer references
// returns it to the pool.
IOSurfaceDecrementUseCount(io_surface);
base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
EXPECT_TRUE(second_buffer);
}
TEST(PixelBufferPoolTest,
SimplyReferencingAnIOSurfaceDoesNotPreventItReturningToThePool) {
constexpr size_t kPoolMaxBuffers = 1;
std::unique_ptr<PixelBufferPool> pool = PixelBufferPool::Create(
kPixelFormatNv12, kVgaWidth, kVgaHeight, kPoolMaxBuffers);
base::ScopedCFTypeRef<CVPixelBufferRef> first_buffer = pool->CreateBuffer();
EXPECT_TRUE(first_buffer);
base::ScopedCFTypeRef<IOSurfaceRef> first_buffer_io_surface(
CVPixelBufferGetIOSurface(first_buffer), base::scoped_policy::RETAIN);
EXPECT_TRUE(first_buffer_io_surface);
CVPixelBufferRef first_buffer_unretaind_ptr = first_buffer;
// Releasing the first buffer returns it to the pool, despite the IOSurface
// still being referenced by |first_buffer_io_surface|.
first_buffer.reset();
base::ScopedCFTypeRef<CVPixelBufferRef> second_buffer = pool->CreateBuffer();
EXPECT_TRUE(second_buffer);
base::ScopedCFTypeRef<IOSurfaceRef> second_buffer_io_surface(
CVPixelBufferGetIOSurface(second_buffer), base::scoped_policy::RETAIN);
EXPECT_TRUE(second_buffer_io_surface);
// Because this is a recycled pixel buffer, the IOSurface is also recycled.
EXPECT_EQ(first_buffer_unretaind_ptr, second_buffer);
EXPECT_EQ(first_buffer_io_surface, second_buffer_io_surface);
}
TEST(PixelBufferPoolTest, BuffersCanOutliveThePool) { TEST(PixelBufferPoolTest, BuffersCanOutliveThePool) {
std::unique_ptr<PixelBufferPool> pool = std::unique_ptr<PixelBufferPool> pool =
PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1); PixelBufferPool::Create(kPixelFormatNv12, kVgaWidth, kVgaHeight, 1);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment