已合并
fix: av buffer queue size_ not locked #2745
linxinyu93创建于 15 天前
fix: av buffer queue size_ not locked #2745
已合并
共 3 个文件变更+181-8
| @@ -1,5 +1,5 @@ | |||
| 1 | /* | 1 | /* |
| 2 | - * Copyright (c) 2021-2025 Huawei Device Co., Ltd. | 2 | + * Copyright (c) 2021-2026 Huawei Device Co., Ltd. |
| 3 | * Licensed under the Apache License, Version 2.0 (the "License"); | 3 | * Licensed under the Apache License, Version 2.0 (the "License"); |
| 4 | * you may not use this file except in compliance with the License. | 4 | * you may not use this file except in compliance with the License. |
| 5 | * You may obtain a copy of the License at | 5 | * You may obtain a copy of the License at |
| @@ -104,18 +104,26 @@ AVBufferQueueImpl::AVBufferQueueImpl(uint32_t size, MemoryType type, const std:: | |||
| 104 | 104 | ||
| 105 | uint32_t AVBufferQueueImpl::GetQueueSize() | 105 | uint32_t AVBufferQueueImpl::GetQueueSize() |
| 106 | { | 106 | { |
| 107 | + std::lock_guard<std::mutex> lockGuard(queueMutex_); | ||
| 107 | return size_; | 108 | return size_; |
| 108 | } | 109 | } |
| 109 | 110 | ||
| 110 | Status AVBufferQueueImpl::SetQueueSize(uint32_t size) | 111 | Status AVBufferQueueImpl::SetQueueSize(uint32_t size) |
| 111 | { | 112 | { |
| 113 | + std::lock_guard<std::mutex> lockGuard(queueMutex_); | ||
| 112 | FALSE_RETURN_V(size >= 0 && size <= AVBUFFER_QUEUE_MAX_QUEUE_SIZE && size != size_, | 114 | FALSE_RETURN_V(size >= 0 && size <= AVBUFFER_QUEUE_MAX_QUEUE_SIZE && size != size_, |
| 113 | Status::ERROR_INVALID_BUFFER_SIZE); | 115 | Status::ERROR_INVALID_BUFFER_SIZE); |
| 114 | 116 | ||
| 115 | - return SetLargerQueueSize(size); | 117 | + return SetLargerQueueSizeLocked(size); |
| 116 | } | 118 | } |
| 117 | 119 | ||
| 118 | Status AVBufferQueueImpl::SetLargerQueueSize(uint32_t size) | 120 | Status AVBufferQueueImpl::SetLargerQueueSize(uint32_t size) |
| 121 | +{ | ||
| 122 | + std::lock_guard<std::mutex> lockGuard(queueMutex_); | ||
| 123 | + return SetLargerQueueSizeLocked(size); | ||
| 124 | +} | ||
| 125 | + | ||
| 126 | +Status AVBufferQueueImpl::SetLargerQueueSizeLocked(uint32_t size) | ||
| 119 | { | 127 | { |
| 120 | FALSE_RETURN_V(size >= 0 && size <= AVBUFFER_QUEUE_MAX_QUEUE_SIZE_FOR_LARGER && size != size_, | 128 | FALSE_RETURN_V(size >= 0 && size <= AVBUFFER_QUEUE_MAX_QUEUE_SIZE_FOR_LARGER && size != size_, |
| 121 | Status::ERROR_INVALID_BUFFER_SIZE); | 129 | Status::ERROR_INVALID_BUFFER_SIZE); |
| @@ -126,7 +134,6 @@ Status AVBufferQueueImpl::SetLargerQueueSize(uint32_t size) | |||
| 126 | requestCondition.notify_all(); | 134 | requestCondition.notify_all(); |
| 127 | } | 135 | } |
| 128 | } else { | 136 | } else { |
| 129 | - std::lock_guard<std::mutex> lockGuard(queueMutex_); | ||
| 130 | DeleteBuffers(size_ - size); | 137 | DeleteBuffers(size_ - size); |
| 131 | size_ = size; | 138 | size_ = size; |
| 132 | } | 139 | } |
| @@ -315,7 +322,7 @@ bool AVBufferQueueImpl::wait_for(std::unique_lock<std::mutex>& lock, int64_t tim | |||
| 315 | if (timeoutUs > 0) { | 322 | if (timeoutUs > 0) { |
| 316 | return requestCondition.wait_for( | 323 | return requestCondition.wait_for( |
| 317 | lock, std::chrono::microseconds(timeoutUs), [this]() { | 324 | lock, std::chrono::microseconds(timeoutUs), [this]() { |
| 318 | - return !freeBufferList_.empty() || (GetCachedBufferCount() < GetQueueSize()); | 325 | + return !freeBufferList_.empty() || (GetCachedBufferCount() < size_); |
| 319 | }); | 326 | }); |
| 320 | } else if (timeoutUs < 0) { | 327 | } else if (timeoutUs < 0) { |
| 321 | requestCondition.wait(lock); | 328 | requestCondition.wait(lock); |
| @@ -351,7 +358,7 @@ Status AVBufferQueueImpl::RequestBufferWaitUs( | |||
| 351 | } | 358 | } |
| 352 | 359 | ||
| 353 | // check queue size | 360 | // check queue size |
| 354 | - if (GetCachedBufferCount() >= GetQueueSize()) { | 361 | + if (GetCachedBufferCount() >= size_) { |
| 355 | if (!wait_for(lock, timeoutUs)) { | 362 | if (!wait_for(lock, timeoutUs)) { |
| 356 | MEDIA_LOG_D("FALSE_RETURN_V wait_for(lock, timeoutUs)"); | 363 | MEDIA_LOG_D("FALSE_RETURN_V wait_for(lock, timeoutUs)"); |
| 357 | return Status::ERROR_WAIT_TIMEOUT; | 364 | return Status::ERROR_WAIT_TIMEOUT; |
| @@ -361,7 +368,7 @@ Status AVBufferQueueImpl::RequestBufferWaitUs( | |||
| 361 | if (ret == Status::OK) { | 368 | if (ret == Status::OK) { |
| 362 | return RequestReuseBuffer(buffer, configCopy); | 369 | return RequestReuseBuffer(buffer, configCopy); |
| 363 | } | 370 | } |
| 364 | - if (GetCachedBufferCount() >= GetQueueSize()) { | 371 | + if (GetCachedBufferCount() >= size_) { |
| 365 | return Status::ERROR_NO_FREE_BUFFER; | 372 | return Status::ERROR_NO_FREE_BUFFER; |
| 366 | } | 373 | } |
| 367 | } | 374 | } |
| @@ -551,7 +558,7 @@ Status AVBufferQueueImpl::AttachAvailableBufferLocked(std::shared_ptr<AVBuffer>& | |||
| 551 | }; | 558 | }; |
| 552 | 559 | ||
| 553 | auto cachedCount = GetCachedBufferCount(); | 560 | auto cachedCount = GetCachedBufferCount(); |
| 554 | - auto queueSize = GetQueueSize(); | 561 | + auto queueSize = size_; |
| 555 | if (cachedCount >= queueSize) { | 562 | if (cachedCount >= queueSize) { |
| 556 | auto validCount = static_cast<uint32_t>(dirtyBufferList_.size() + freeBufferList_.size()); | 563 | auto validCount = static_cast<uint32_t>(dirtyBufferList_.size() + freeBufferList_.size()); |
| 557 | auto toBeDeleteCount = cachedCount - queueSize; | 564 | auto toBeDeleteCount = cachedCount - queueSize; |
| @@ -1,5 +1,5 @@ | |||
| 1 | /* | 1 | /* |
| 2 | - * Copyright (c) 2021-2025 Huawei Device Co., Ltd. | 2 | + * Copyright (c) 2021-2026 Huawei Device Co., Ltd. |
| 3 | * Licensed under the Apache License, Version 2.0 (the "License"); | 3 | * Licensed under the Apache License, Version 2.0 (the "License"); |
| 4 | * you may not use this file except in compliance with the License. | 4 | * you may not use this file except in compliance with the License. |
| 5 | * You may obtain a copy of the License at | 5 | * You may obtain a copy of the License at |
| @@ -124,6 +124,7 @@ private: | |||
| 124 | Status AttachAvailableBufferLocked(std::shared_ptr<AVBuffer>& buffer); | 124 | Status AttachAvailableBufferLocked(std::shared_ptr<AVBuffer>& buffer); |
| 125 | Status PushBufferOnFilled(uint64_t uniqueId, bool isFilled); | 125 | Status PushBufferOnFilled(uint64_t uniqueId, bool isFilled); |
| 126 | void SetQueueSizeBeforeAttachBufferLocked(uint32_t size); | 126 | void SetQueueSizeBeforeAttachBufferLocked(uint32_t size); |
| 127 | + Status SetLargerQueueSizeLocked(uint32_t size); | ||
| 127 | uint32_t size_; | 128 | uint32_t size_; |
| 128 | MemoryType memoryType_; | 129 | MemoryType memoryType_; |
| 129 | bool disableAlloc_; | 130 | bool disableAlloc_; |
| @@ -13,6 +13,8 @@ | |||
| 13 | * limitations under the License. | 13 | * limitations under the License. |
| 14 | */ | 14 | */ |
| 15 | 15 | ||
| 16 | + | ||
| 17 | + | ||
| 16 | 18 | ||
| 17 | 19 | ||
| 18 | 20 | ||
| @@ -550,6 +552,169 @@ HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_001, TestSize.Level1) | |||
| 550 | EXPECT_EQ(avBufferQueueImpl_->GetFilledBufferSize(), 0); | 552 | EXPECT_EQ(avBufferQueueImpl_->GetFilledBufferSize(), 0); |
| 551 | } | 553 | } |
| 552 | 554 | ||
| 555 | +/** | ||
| 556 | + * @tc.name: RequestBufferWaitUs_002 | ||
| 557 | + * @tc.desc: Test RequestBufferWaitUs - free buffer available, reuse from free list directly (line 355-357) | ||
| 558 | + * @tc.type: FUNC | ||
| 559 | + */ | ||
| 560 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_002, TestSize.Level1) | ||
| 561 | +{ | ||
| 562 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(2), Status::OK); | ||
| 563 | + AVBufferConfig config; | ||
| 564 | + config.size = 100; | ||
| 565 | + config.capacity = 100; | ||
| 566 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 567 | + | ||
| 568 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 569 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 570 | + avBufferQueueImpl_->InsertFreeBufferInOrder(buffer1->GetUniqueId()); | ||
| 571 | + | ||
| 572 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 573 | + EXPECT_EQ(avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 0), Status::OK); | ||
| 574 | + EXPECT_NE(reqBuffer, nullptr); | ||
| 575 | + EXPECT_EQ(reqBuffer->GetUniqueId(), buffer1->GetUniqueId()); | ||
| 576 | +} | ||
| 577 | + | ||
| 578 | +/** | ||
| 579 | + * @tc.name: RequestBufferWaitUs_003 | ||
| 580 | + * @tc.desc: Test RequestBufferWaitUs - queue not full, allocate new buffer directly (line 361 false, 376) | ||
| 581 | + * @tc.type: FUNC | ||
| 582 | + */ | ||
| 583 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_003, TestSize.Level1) | ||
| 584 | +{ | ||
| 585 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(2), Status::OK); | ||
| 586 | + AVBufferConfig config; | ||
| 587 | + config.size = 100; | ||
| 588 | + config.capacity = 100; | ||
| 589 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 590 | + | ||
| 591 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 592 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 593 | + | ||
| 594 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 595 | + EXPECT_EQ(avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 0), Status::OK); | ||
| 596 | + EXPECT_NE(reqBuffer, nullptr); | ||
| 597 | + EXPECT_NE(reqBuffer->GetUniqueId(), buffer1->GetUniqueId()); | ||
| 598 | +} | ||
| 599 | + | ||
| 600 | +/** | ||
| 601 | + * @tc.name: RequestBufferWaitUs_004 | ||
| 602 | + * @tc.desc: Test RequestBufferWaitUs - queue full, wait_for timeout (line 361 true, 362 -> ERROR_WAIT_TIMEOUT) | ||
| 603 | + * @tc.type: FUNC | ||
| 604 | + */ | ||
| 605 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_004, TestSize.Level1) | ||
| 606 | +{ | ||
| 607 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(1), Status::OK); | ||
| 608 | + AVBufferConfig config; | ||
| 609 | + config.size = 100; | ||
| 610 | + config.capacity = 100; | ||
| 611 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 612 | + | ||
| 613 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 614 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 615 | + | ||
| 616 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 617 | + EXPECT_EQ(avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 1000), Status::ERROR_WAIT_TIMEOUT); | ||
| 618 | +} | ||
| 619 | + | ||
| 620 | +/** | ||
| 621 | + * @tc.name: RequestBufferWaitUs_005 | ||
| 622 | + * @tc.desc: Test RequestBufferWaitUs - queue full, wait_for returns true immediately (timeoutUs=0), | ||
| 623 | + * still full after wait -> ERROR_NO_FREE_BUFFER (line 361 true, 371 true) | ||
| 624 | + * @tc.type: FUNC | ||
| 625 | + */ | ||
| 626 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_005, TestSize.Level1) | ||
| 627 | +{ | ||
| 628 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(1), Status::OK); | ||
| 629 | + AVBufferConfig config; | ||
| 630 | + config.size = 100; | ||
| 631 | + config.capacity = 100; | ||
| 632 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 633 | + | ||
| 634 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 635 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 636 | + | ||
| 637 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 638 | + EXPECT_EQ(avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 0), Status::ERROR_NO_FREE_BUFFER); | ||
| 639 | +} | ||
| 640 | + | ||
| 641 | +/** | ||
| 642 | + * @tc.name: RequestBufferWaitUs_006 | ||
| 643 | + * @tc.desc: Test RequestBufferWaitUs - queue full, buffer freed during wait, | ||
| 644 | + * reuse after wait (line 361 true, 362 false, 368 true) | ||
| 645 | + * @tc.type: FUNC | ||
| 646 | + */ | ||
| 647 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_006, TestSize.Level1) | ||
| 648 | +{ | ||
| 649 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(1), Status::OK); | ||
| 650 | + AVBufferConfig config; | ||
| 651 | + config.size = 100; | ||
| 652 | + config.capacity = 100; | ||
| 653 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 654 | + | ||
| 655 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 656 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 657 | + uint64_t uniqueId = buffer1->GetUniqueId(); | ||
| 658 | + | ||
| 659 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 660 | + Status result = Status::ERROR_UNKNOWN; | ||
| 661 | + std::thread requestThread([&result, &reqBuffer, &config, this]() { | ||
| 662 | + result = avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 5000000); | ||
| 663 | + }); | ||
| 664 | + | ||
| 665 | + std::thread releaseThread([&uniqueId, this]() { | ||
| 666 | + std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||
| 667 | + { | ||
| 668 | + std::lock_guard<std::mutex> lock(avBufferQueueImpl_->queueMutex_); | ||
| 669 | + avBufferQueueImpl_->InsertFreeBufferInOrder(uniqueId); | ||
| 670 | + avBufferQueueImpl_->requestCondition.notify_all(); | ||
| 671 | + } | ||
| 672 | + }); | ||
| 673 | + | ||
| 674 | + releaseThread.join(); | ||
| 675 | + requestThread.join(); | ||
| 676 | + | ||
| 677 | + EXPECT_EQ(result, Status::OK); | ||
| 678 | + EXPECT_NE(reqBuffer, nullptr); | ||
| 679 | + EXPECT_EQ(reqBuffer->GetUniqueId(), uniqueId); | ||
| 680 | +} | ||
| 681 | + | ||
| 682 | +/** | ||
| 683 | + * @tc.name: RequestBufferWaitUs_007 | ||
| 684 | + * @tc.desc: Test RequestBufferWaitUs - queue full, size increased during wait, | ||
| 685 | + * allocate new buffer after wait (line 361 true, 362 false, 371 false -> 376) | ||
| 686 | + * @tc.type: FUNC | ||
| 687 | + */ | ||
| 688 | +HWTEST_F(AVBufferQueueInnerUnitTest, RequestBufferWaitUs_007, TestSize.Level1) | ||
| 689 | +{ | ||
| 690 | + ASSERT_EQ(avBufferQueueImpl_->SetQueueSize(1), Status::OK); | ||
| 691 | + AVBufferConfig config; | ||
| 692 | + config.size = 100; | ||
| 693 | + config.capacity = 100; | ||
| 694 | + config.memoryType = MemoryType::VIRTUAL_MEMORY; | ||
| 695 | + | ||
| 696 | + std::shared_ptr<AVBuffer> buffer1; | ||
| 697 | + ASSERT_EQ(avBufferQueueImpl_->AllocBuffer(buffer1, config), Status::OK); | ||
| 698 | + | ||
| 699 | + std::shared_ptr<AVBuffer> reqBuffer; | ||
| 700 | + Status result = Status::ERROR_UNKNOWN; | ||
| 701 | + std::thread requestThread([&result, &reqBuffer, &config, this]() { | ||
| 702 | + result = avBufferQueueImpl_->RequestBufferWaitUs(reqBuffer, config, 5000000); | ||
| 703 | + }); | ||
| 704 | + | ||
| 705 | + std::thread resizeThread([this]() { | ||
| 706 | + std::this_thread::sleep_for(std::chrono::milliseconds(100)); | ||
| 707 | + EXPECT_EQ(avBufferQueueImpl_->SetQueueSize(2), Status::OK); | ||
| 708 | + }); | ||
| 709 | + | ||
| 710 | + resizeThread.join(); | ||
| 711 | + requestThread.join(); | ||
| 712 | + | ||
| 713 | + EXPECT_EQ(result, Status::OK); | ||
| 714 | + EXPECT_NE(reqBuffer, nullptr); | ||
| 715 | + EXPECT_NE(reqBuffer->GetUniqueId(), buffer1->GetUniqueId()); | ||
| 716 | +} | ||
| 717 | + | ||
| 553 | /** | 718 | /** |
| 554 | * @tc.name: ProducerProxyCreate_001 | 719 | * @tc.name: ProducerProxyCreate_001 |
| 555 | * @tc.desc: Test ProducerProxyCreate interface | 720 | * @tc.desc: Test ProducerProxyCreate interface |