[media] Add test to ensure CodecImpl's BufferCollections close cleanly
Due to the addition of `EnsureBuffersNotConfigured()` in the relevant
code paths (see commit d04382d70c0), it appears that Bug 37257 was
already resolved.
Added a test to ensure this behaviour persists, and also removed some
dead code.
Bug: 37257
Test: fx run-test decryptor_adapter_tests
Change-Id: Icf2c7da4fe1c20c6b9d323c1479ec38d1face5c7
diff --git a/src/media/lib/codec_impl/codec_impl.cc b/src/media/lib/codec_impl/codec_impl.cc
index 007390c..a4ed6be 100644
--- a/src/media/lib/codec_impl/codec_impl.cc
+++ b/src/media/lib/codec_impl/codec_impl.cc
@@ -1348,15 +1348,10 @@
EnsureBuffersNotConfigured(lock, kOutputPort);
- // This unbinds any BufferCollection bindings. This is effectively part
- // of the overall unbind of anything generating activity on FIDL thread
- // based on incoming channel messages.
- if (port_settings_[kInputPort] && port_settings_[kInputPort]->is_partial_settings()) {
- port_settings_[kInputPort]->UnbindBufferCollection();
- }
- if (port_settings_[kOutputPort] && port_settings_[kOutputPort]->is_partial_settings()) {
- port_settings_[kOutputPort]->UnbindBufferCollection();
- }
+ // By this point both PortSettings should have already been deleted.
+ ZX_DEBUG_ASSERT(!port_settings_[kInputPort]);
+ ZX_DEBUG_ASSERT(!port_settings_[kOutputPort]);
+
// Unbind the sysmem_ fuchsia::sysmem::Allocator connection - this also
// ensures that any in-flight requests' completions will not run.
sysmem_.Unbind();
diff --git a/src/media/lib/codec_impl/unit_tests/test_decryptor_adapter.cc b/src/media/lib/codec_impl/unit_tests/test_decryptor_adapter.cc
index 2ff4c47..83009c3 100644
--- a/src/media/lib/codec_impl/unit_tests/test_decryptor_adapter.cc
+++ b/src/media/lib/codec_impl/unit_tests/test_decryptor_adapter.cc
@@ -543,6 +543,55 @@
AssertNoChannelErrors();
}
+TEST_F(ClearDecryptorAdapterTest, DecryptorClosesBuffersCleanly) {
+ ConnectDecryptor(false);
+ decryptor_adapter_->set_has_keys(true);
+
+ RunLoopUntil([this]() { return input_buffer_info_.has_value(); });
+ AssertNoChannelErrors();
+ ASSERT_TRUE(input_buffer_info_);
+
+ ConfigureInputPackets();
+
+ decryptor_->QueueInputFormatDetails(kStreamLifetimeOrdinal,
+ CreateInputFormatDetails("clear", {}, {}));
+
+ // Queue a single input packet.
+ ASSERT_TRUE(HasFreePackets());
+ decryptor_->QueueInputPacket(CreateInputPacket(*input_iter_++));
+
+ // Wait until the output collection has been allocated.
+ RunLoopUntil([this]() { return output_buffer_info_.has_value(); });
+ AssertNoChannelErrors();
+ ASSERT_TRUE(output_buffer_info_);
+
+ // Wait until receive first output packet.
+ RunLoopUntil([this]() { return !output_data_.empty(); });
+ AssertNoChannelErrors();
+
+ // Drop the decryptor_. This should not cause any buffer collection failures.
+ decryptor_ = nullptr;
+ AssertNoChannelErrors();
+
+ // If the below fail, the collections have failed.
+ std::optional<zx_status_t> input_status;
+ std::optional<zx_status_t> output_status;
+ EXPECT_TRUE(input_collection_.is_bound());
+ EXPECT_TRUE(output_collection_.is_bound());
+ input_collection_->CheckBuffersAllocated([&input_status](zx_status_t s) { input_status = s; });
+ output_collection_->CheckBuffersAllocated([&output_status](zx_status_t s) { output_status = s; });
+ RunLoopUntil([&]() { return input_status.has_value() && output_status.has_value(); });
+ EXPECT_EQ(*input_status, ZX_OK);
+ EXPECT_EQ(*output_status, ZX_OK);
+ AssertNoChannelErrors();
+ EXPECT_FALSE(stream_error_.has_value());
+
+ // Buffers are A-OK after dropping decryptor_.
+
+ // ClearText decryptor just copies data across, and only one input packet was sent.
+ EXPECT_EQ(output_data_[0], input_data_[0]);
+}
+
class SecureDecryptorAdapterTest : public DecryptorAdapterTest<FakeSecureDecryptorAdapter> {};
TEST_F(SecureDecryptorAdapterTest, FailsToAcquireSecureBuffers) {
