Use GUARDED_BY in VaapiWrapper |
|||||||
Issue descriptionGUARDED_BY is available to use in chromium code base and recommended to use [1]. VaapiWrapper is a heavy user to lock, va_lock*. We should statically check the correctness of lock status on each function. NOTE: GUARDED_BY apparently doesn't work with base::AutoUnlock(). We should think we are able to get rid of base::AutoUnlock in VaapiWrapper. [1] https://groups.google.com/a/chromium.org/forum/?utm_medium=email&utm_source=footer#!msg/chromium-dev/mG5sfyU1GGk/uo9ht2PsAgAJ
,
Nov 28
Miguel, I assign you wondering whether you have bandwidth for this.
,
Nov 29
Good idea Hiro! Only just discovered the existence of the 'GUARDED_BY' annotation, it's amazing! This change looks straightforward enough. If you want I'll take it up!
,
Nov 29
Why would we need to get rid of base::AutoUnlock? The GUARDED_BY macro checks whether variables are only accessed when the lock is acquired, it doesn't check whether the lock is released in all other cases. So the right thing the GUARDED_BY macro should do is ignore the use of 'AutoUnlock'. I quickly added some GUARDED_BY macros, to see what would happen in: http://crrev.com/c/1353063. As expected it throws out tons of disturbing warnings. I guess the important thing we want here, is that all calls to the underlying API are serialized. As we can't use the macro on functions we need to add it to variables that are used in these function calls. va_display_: All access to this var should definitely be protected by a lock, as it's used in most API calls. It throws out tons of warnings, e.g. in vaCreateConfig(). va_config_id_: Similar to the above va_surface_ids_: Same as above va_surface_format_: Not sure if this one need to be protected, maybe to be on the safe side. After Hiro's CL a lot of these warnings will disappear? I'll wait for the CL to get submitted and see if I can fix the remaining errors.
,
Nov 29
Thanks david for quick experiment. Let's fix together next week by pair-programming.
,
Nov 29
+acourbot@, who may be interested in GUARDED_BY introduction in vaapi wrapper.
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04fbdd9f175a350b7a674ab0610c3963428e0e1d commit 04fbdd9f175a350b7a674ab0610c3963428e0e1d Author: Hirokazu Honda <hiroh@chromium.org> Date: Thu Nov 29 08:31:34 2018 media/gpu/vaapi/vaapiWrapper: Acquire lock in CreateContext() and DestroySurfaces() CreateContext() doesn't acquire lock if it is called by VaapiWrapper's caller. Lock should be acquired always in CreateContext(). This change also removes DestroySurfaces_Locked() and instead DestroySurfaces() is called everywhere. CreateSurfaces() and DestroySurfaces() are renamed to CreateContextAndSurfaces() and DestroyContextAndSurfaces() to specify what they are doing. Bug: 909547 , 909561 Test: VDA, VEA, JDA, JEA unittest on eve Change-Id: Ib04386307c2e83d83e0afe079a1868ac02eff086 Reviewed-on: https://chromium-review.googlesource.com/c/1353056 Commit-Queue: Hirokazu Honda <hiroh@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#612101} [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_jpeg_decode_accelerator.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_jpeg_encode_accelerator.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_video_encode_accelerator.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_wrapper.cc [modify] https://crrev.com/04fbdd9f175a350b7a674ab0610c3963428e0e1d/media/gpu/vaapi/vaapi_wrapper.h
,
Dec 6
https://bugs.chromium.org/p/chromium/issues/detail?id=881875#c15 I see that a pattern e.g. Bla input_buffers_ GUARDED_BY(lock_); and lock_.AssertAcquired(); // ... input_buffers_.pop(); causes ... : error: reading variable 'input_buffers_' requires holding mutex 'lock_' [-Werror,-Wthread-safety-analysis] input_buffers_.pop();
,
Dec 6
I suspect that you need to add EXCLUSIVE_LOCKS_REQUIRED(lock_) annotation to the function calling |lock_.AssertAcquired()| - similarly how this was done in r593981 - see the changes that r593981 made in plugin_list.h and see the call to lock_.AssertAcquired() in PluginList::RemoveExtraPluginPathLocked. OTOH, I am only guessing that the suggestion above will help - I don't have enough info to be confident in that suggestion (e.g. I don't know where |input_buffers_.pop();| is).
,
Dec 6
mcasas@, so - which bug should we use going forward to discuss the issue you've run into? You made the same comment in https://crbug.com/881875#c15 and https://crbug.com/909547#c8 and now you have two people separately giving back the same advice... :-/
,
Dec 7
lukasza@, I just tried your suggestion and it was spot on, the EXCLUSIVE_LOCKS_REQUIRED() annotation made it understand that the |input_buffers_| was going to be protected. Thanks !!
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/99e2d9e3cdba89a15ebd4890b2de37336963a0b0 commit 99e2d9e3cdba89a15ebd4890b2de37336963a0b0 Author: Miguel Casas-Sanchez <mcasas@chromium.org> Date: Thu Dec 13 03:38:03 2018 Vaapi: Use GUARDED_BY() static annotations on decoder This CL introduces GUARDED_BY() static annotation on VaVDA; not all members variables need to be guarded, so it adds on a number of them a comment about on which thread it is used. Bug: 909547 Change-Id: I437a82320795aebc56f77cd1254bb7c8c2f2f5da Reviewed-on: https://chromium-review.googlesource.com/c/1368590 Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#616196} [modify] https://crrev.com/99e2d9e3cdba89a15ebd4890b2de37336963a0b0/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/99e2d9e3cdba89a15ebd4890b2de37336963a0b0/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/99e2d9e3cdba89a15ebd4890b2de37336963a0b0/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/018fa575f326ce8a1d9173b3705f40f0771713f3 commit 018fa575f326ce8a1d9173b3705f40f0771713f3 Author: David Staessens <dstaessens@chromium.org> Date: Thu Dec 13 04:34:05 2018 media/gpu/vaapi: Use GUARDED_BY annotation in the Vaapi Wrapper. This CL adds the necessary checks to make sure that all access to the underlying VAAPI library is thread-safe. TEST=ran VDA and JDA tests on Nocturne BUG= 909547 Change-Id: Ibf1ee6a8f2d07e31c1142eb7629e9c0e903d00a8 Reviewed-on: https://chromium-review.googlesource.com/c/1353063 Commit-Queue: David Staessens <dstaessens@chromium.org> Reviewed-by: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#616215} [modify] https://crrev.com/018fa575f326ce8a1d9173b3705f40f0771713f3/media/gpu/vaapi/vaapi_jpeg_decode_accelerator_unittest.cc [modify] https://crrev.com/018fa575f326ce8a1d9173b3705f40f0771713f3/media/gpu/vaapi/vaapi_wrapper.cc [modify] https://crrev.com/018fa575f326ce8a1d9173b3705f40f0771713f3/media/gpu/vaapi/vaapi_wrapper.h
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/57d51cc17e0e2c806d1b03eb6ce0905ed25f4cda commit 57d51cc17e0e2c806d1b03eb6ce0905ed25f4cda Author: Hirokazu Honda <hiroh@chromium.org> Date: Fri Dec 14 00:34:41 2018 Revert "Vaapi: Use GUARDED_BY() static annotations on decoder" This reverts commit 99e2d9e3cdba89a15ebd4890b2de37336963a0b0. Reason for revert: This breaks Video Playback on intel devices Original change's description: > Vaapi: Use GUARDED_BY() static annotations on decoder > > This CL introduces GUARDED_BY() static annotation on VaVDA; not all > members variables need to be guarded, so it adds on a number of them > a comment about on which thread it is used. > > Bug: 909547 > Change-Id: I437a82320795aebc56f77cd1254bb7c8c2f2f5da > Reviewed-on: https://chromium-review.googlesource.com/c/1368590 > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Commit-Queue: Miguel Casas <mcasas@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616196} TBR=mcasas@chromium.org,hiroh@chromium.org,dstaessens@chromium.org Change-Id: I2e6496e9148f12d37f746a7d0b1e0e26cee70341 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 909547 , 914954 Reviewed-on: https://chromium-review.googlesource.com/c/1377470 Commit-Queue: Hirokazu Honda <hiroh@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Cr-Commit-Position: refs/heads/master@{#616522} [modify] https://crrev.com/57d51cc17e0e2c806d1b03eb6ce0905ed25f4cda/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/57d51cc17e0e2c806d1b03eb6ce0905ed25f4cda/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/57d51cc17e0e2c806d1b03eb6ce0905ed25f4cda/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc
,
Dec 14
Miguel, I reverted crrev.com/c/1368590 because it causes video playback breakage on intel devices. The problem is reproducible to play video on crosvideo.appspot.com. Could you take a look when you reland it? Thanks.
,
Dec 14
Running the video_decode_accelerator_unittests seems to run into a deadlock: [ RUN ] ResourceExhaustion/VideoDecodeAcceleratorParamTest.TestSimpleDecode/1 [13010:13014:1212/235931.839574:198195839583:FATAL:lock_impl_posix.cc(103)] Check failed: rv == 0 (35 vs. 0). Resource deadlock avoided. #0 0x555b4aa7d328 base::debug::StackTrace::StackTrace() #1 0x555b4a84719c base::debug::StackTrace::StackTrace() #2 0x555b4a87e07a logging::LogMessage::~LogMessage() #3 0x555b4aaa06ea base::internal::LockImpl::Lock() #4 0x555b4825be73 base::Lock::Acquire() #5 0x555b4825be03 base::AutoLock::AutoLock() #6 0x555b4a2a5eab media::VaapiVideoDecodeAccelerator::OutputPicture() #7 0x555b4a2be952 _ZN4base8internal13FunctorTraitsIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS2_9VASurfaceEEiN3gfx4RectERKNS2_15VideoColorSpaceEEvE6InvokeISF_NS_7WeakPtrIS3_EEJS6_iSA_SB_EEEvT_OT0_DpOT1_ #8 0x555b4a2be847 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS4_9VASurfaceEEiN3gfx4RectERKNS4_15VideoColorSpaceEENS_7WeakPtrIS5_EEJS8_iSC_SD_EEEvOT_OT0_DpOT1_ #9 0x555b4a2be77d _ZN4base8internal7InvokerINS0_9BindStateIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS3_9VASurfaceEEiN3gfx4RectERKNS3_15VideoColorSpaceEEJNS_7WeakPtrIS4_EES7_iSB_SC_EEEFvvEE7RunImplISG_NSt3__15tupleIJSI_S7_iSB_SC_EEEJLm0ELm1ELm2ELm3ELm4EEEEvOT_OT0_NSN_16integer_sequenceImJXspT1_EEEE #10 0x555b4a2be569 _ZN4base8internal7InvokerINS0_9BindStateIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS3_9VASurfaceEEiN3gfx4RectERKNS3_15VideoColorSpaceEEJNS_7WeakPtrIS4_EES7_iSB_SC_EEEFvvEE7RunOnceEPNS0_13BindStateBaseE #11 0x555b482db92c _ZNO4base12OnceCallbackIFvvEE3RunEv #12 0x555b4a2a6f01 media::VaapiVideoDecodeAccelerator::TryOutputPicture() #13 0x555b4a2aeff3 media::VaapiVideoDecodeAccelerator::SurfaceReady() #14 0x555b4a2bdcf8 _ZN4base8internal13FunctorTraitsIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS2_9VASurfaceEEiRKN3gfx4RectERKNS2_15VideoColorSpaceEEvE6InvokeISH_RKNS_7WeakPtrIS3_EEJS8_RKiSC_SF_EEEvT_OT0_DpOT1_ #15 0x555b4a2bdc27 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS4_9VASurfaceEEiRKN3gfx4RectERKNS4_15VideoColorSpaceEERKNS_7WeakPtrIS5_EEJSA_RKiSE_SH_EEEvOT_OT0_DpOT1_ #16 0x555b4a2bdb5d _ZN4base8internal7InvokerINS0_9BindStateIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS3_9VASurfaceEEiRKN3gfx4RectERKNS3_15VideoColorSpaceEEJNS_7WeakPtrIS4_EES7_iSB_SE_EEEFvvEE7RunImplIRKSI_RKNSt3__15tupleIJSK_S7_iSB_SE_EEEJLm0ELm1ELm2ELm3ELm4EEEEvOT_OT0_NSR_16integer_sequenceImJXspT1_EEEE #17 0x555b4a2bd93c _ZN4base8internal7InvokerINS0_9BindStateIMN5media27VaapiVideoDecodeAcceleratorEFvRK13scoped_refptrINS3_9VASurfaceEEiRKN3gfx4RectERKNS3_15VideoColorSpaceEEJNS_7WeakPtrIS4_EES7_iSB_SE_EEEFvvEE3RunEPNS0_13BindStateBaseE #18 0x555b482db92c _ZNO4base12OnceCallbackIFvvEE3RunEv #19 0x555b4aadcf38 base::debug::TaskAnnotator::RunTask() #20 0x555b4a89261a base::MessageLoopImpl::RunTask() #21 0x555b4a8929ce base::MessageLoopImpl::DeferOrRunPendingTask() #22 0x555b4a8933b9 base::MessageLoopImpl::DoWork() #23 0x555b4aab6085 base::MessagePumpLibevent::Run() #24 0x555b4a891d43 base::MessageLoopImpl::Run() #25 0x555b4a90761d base::RunLoop::Run() #26 0x555b4a9cd508 base::Thread::Run() #27 0x555b4a9ce0e3 base::Thread::ThreadMain() #28 0x555b4aaab186 base::(anonymous namespace)::ThreadFunc() #29 0x7cfe606804fe <unknown> #30 0x7cfe5fb7fbef clone [12698:12701:1212/235932.014532:198196014546:ERROR:kill_posix.cc(84)] Unable to terminate process group 13010: No such process (3) [31/40] ResourceExhaustion/VideoDecodeAcceleratorParamTest.TestSimpleDecode/1 (CRASHED) [13022:13022:1212/235932.214757:198196214859:ERROR:icu_util.cc(172)] Invalid file descriptor to ICU data received. Note: Google Test filter = ResourceExhaustion/VideoDecodeAcceleratorParamTest.TestSimpleDecode/0 It seems fixed after the revert.
,
Dec 14
Created crrev.com/c/1377479 that fixes the deadlocks.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c commit b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c Author: David Staessens <dstaessens@chromium.org> Date: Wed Dec 19 10:14:13 2018 Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder" Deadlocks in the original code are fixed now. TEST=ran VDA tests on Nocturne Bug= 909547 > Vaapi: Use GUARDED_BY() static annotations on decoder > > This CL introduces GUARDED_BY() static annotation on VaVDA; not all > members variables need to be guarded, so it adds on a number of them > a comment about on which thread it is used. > Bug: 909547 > Change-Id: I437a82320795aebc56f77cd1254bb7c8c2f2f5da > Reviewed-on: https://chromium-review.googlesource.com/c/1368590 > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Commit-Queue: Miguel Casas <mcasas@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616196} Change-Id: If37bfd2d1f24b9b833bf8b291363b5c19861a6c7 Reviewed-on: https://chromium-review.googlesource.com/c/1377479 Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Hirokazu Honda <hiroh@chromium.org> Cr-Commit-Position: refs/heads/master@{#617781} [modify] https://crrev.com/b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cb5feb064ec4a02a8591e59c61e44920d7455a47 commit cb5feb064ec4a02a8591e59c61e44920d7455a47 Author: Hirokazu Honda <hiroh@chromium.org> Date: Thu Dec 20 02:44:26 2018 Revert "Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder"" This reverts commit b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c. Reason for revert: This causes inifite-loop in IMPORT mode. Original change's description: > Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder" > > Deadlocks in the original code are fixed now. > > TEST=ran VDA tests on Nocturne > > Bug= 909547 > > > Vaapi: Use GUARDED_BY() static annotations on decoder > > > > This CL introduces GUARDED_BY() static annotation on VaVDA; not all > > members variables need to be guarded, so it adds on a number of them > > a comment about on which thread it is used. > > > Bug: 909547 > > Change-Id: I437a82320795aebc56f77cd1254bb7c8c2f2f5da > > Reviewed-on: https://chromium-review.googlesource.com/c/1368590 > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > Commit-Queue: Miguel Casas <mcasas@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#616196} > > Change-Id: If37bfd2d1f24b9b833bf8b291363b5c19861a6c7 > Reviewed-on: https://chromium-review.googlesource.com/c/1377479 > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Commit-Queue: Hirokazu Honda <hiroh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#617781} TBR=mcasas@chromium.org,hiroh@chromium.org,dstaessens@chromium.org Change-Id: Ic4b1e7e8de8403173f71a415094ed23cab51f5e4 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/1385689 Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Hirokazu Honda <hiroh@chromium.org> Cr-Commit-Position: refs/heads/master@{#618085} [modify] https://crrev.com/cb5feb064ec4a02a8591e59c61e44920d7455a47/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/cb5feb064ec4a02a8591e59c61e44920d7455a47/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/cb5feb064ec4a02a8591e59c61e44920d7455a47/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/de13353df3f9dbc253062fb35c184f967ba721b3 commit de13353df3f9dbc253062fb35c184f967ba721b3 Author: Hirokazu Honda <hiroh@chromium.org> Date: Thu Dec 20 04:41:42 2018 Revert "RELAND: Vaapi decode: split |decoder_|s GetRequiredNumOfPictures()" This reverts commit f3a251c4ae81ebb23a95131e040c217d022b3a93 (crrev.com/c/1379274) Reason for revert: This causes GTS breakage TBR=mcasas@chromium.org Bug: 909547 Bug: b:121169667, b:121003733 Test: GtsExoPlayerTestCases Change-Id: I949843abf926488e152f215a26391b3d08ba4be5 Reviewed-on: https://chromium-review.googlesource.com/c/1385692 Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Commit-Queue: Hirokazu Honda <hiroh@chromium.org> Cr-Commit-Position: refs/heads/master@{#618112} [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/accelerated_video_decoder.h [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/h264_decoder.cc [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/h264_decoder.h [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vp8_decoder.cc [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vp8_decoder.h [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vp9_decoder.cc [modify] https://crrev.com/de13353df3f9dbc253062fb35c184f967ba721b3/media/gpu/vp9_decoder.h
,
Dec 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c66ed12a80d9dce342314fbc03240cb44cd3f0b commit 5c66ed12a80d9dce342314fbc03240cb44cd3f0b Author: Hirokazu Honda <hiroh@chromium.org> Date: Thu Dec 20 17:26:21 2018 Reland "Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder"" This reverts commit cb5feb064ec4a02a8591e59c61e44920d7455a47. Bug: 909547 Test: CTS and GTS Test: VDA unittest with --test_import Original change's description: > Revert "Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder"" > > This reverts commit b8ad3f6a5f474abbbb395e89eb25cf76ed1f3c5c. > > Reason for revert: This causes inifite-loop in IMPORT mode. > > Original change's description: > > Re-land "Vaapi: Use GUARDED_BY() static annotations on decoder" > > > > Deadlocks in the original code are fixed now. > > > > TEST=ran VDA tests on Nocturne > > > > Bug= 909547 > > > > > Vaapi: Use GUARDED_BY() static annotations on decoder > > > > > > This CL introduces GUARDED_BY() static annotation on VaVDA; not all > > > members variables need to be guarded, so it adds on a number of them > > > a comment about on which thread it is used. > > > > > Bug: 909547 > > > Change-Id: I437a82320795aebc56f77cd1254bb7c8c2f2f5da > > > Reviewed-on: https://chromium-review.googlesource.com/c/1368590 > > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > Commit-Queue: Miguel Casas <mcasas@chromium.org> > > > Cr-Commit-Position: refs/heads/master@{#616196} > > > > Change-Id: If37bfd2d1f24b9b833bf8b291363b5c19861a6c7 > > Reviewed-on: https://chromium-review.googlesource.com/c/1377479 > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > Commit-Queue: Hirokazu Honda <hiroh@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#617781} > > TBR=mcasas@chromium.org,hiroh@chromium.org,dstaessens@chromium.org > > Change-Id: Ic4b1e7e8de8403173f71a415094ed23cab51f5e4 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://chromium-review.googlesource.com/c/1385689 > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > Commit-Queue: Hirokazu Honda <hiroh@chromium.org> > Cr-Commit-Position: refs/heads/master@{#618085} Change-Id: I96f758b3f99032db219ffce1d432c348e6a30e90 Reviewed-on: https://chromium-review.googlesource.com/c/1385728 Reviewed-by: Miguel Casas <mcasas@chromium.org> Commit-Queue: Miguel Casas <mcasas@chromium.org> Cr-Commit-Position: refs/heads/master@{#618257} [modify] https://crrev.com/5c66ed12a80d9dce342314fbc03240cb44cd3f0b/media/gpu/vaapi/vaapi_video_decode_accelerator.cc [modify] https://crrev.com/5c66ed12a80d9dce342314fbc03240cb44cd3f0b/media/gpu/vaapi/vaapi_video_decode_accelerator.h [modify] https://crrev.com/5c66ed12a80d9dce342314fbc03240cb44cd3f0b/media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc
,
Dec 27
Reassigning to dstaessens@ who did most of the above CLs, dstaessens@, can we mark this bug as Fixed now ?
,
Jan 7
While there is always room for improvement, I think we can close this one now :-) |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by hiroh@chromium.org
, Nov 28