JpegEncodeAcceleratorTest.ResolutionChange doesn't work in debug |
|||||||||||||
Issue descriptionRunning the jpeg_encode_accelerator_unnitest in debug results in a crash: [ RUN ] JpegEncodeAcceleratorTest.ResolutionChange [1113/235152.377970:INFO:jpeg_encode_accelerator_unittest.cc(232)] hw_encode_time: 3427 [1113/235152.378011:INFO:jpeg_encode_accelerator_unittest.cc(232)] sw_encode_time: 32748 [1113/235152.387629:FATAL:vaapi_wrapper.cc(893)] Check failed: va_surface_ids_.empty(). #0 0x5a1ae2fb50a8 base::debug::StackTrace::StackTrace() #1 0x5a1ae2dc1efc base::debug::StackTrace::StackTrace() #2 0x5a1ae2df8cba logging::LogMessage::~LogMessage() #3 0x5a1ae2993c90 media::VaapiWrapper::CreateSurfaces() #4 0x5a1ae294bf06 media::VaapiJpegEncodeAccelerator::Encoder::EncodeTask() #5 0x5a1ae2955667 _ZN4base8internal13FunctorTraitsIMN5media26VaapiJpegEncodeAccelerator7EncoderEFvNSt3__110unique_ptrINS3_13EncodeRequestENS5_14default_deleteIS7_EEEEEvE6InvokeISC_PS4_JSA_EEEvT_OT0_DpOT1_ #6 0x5a1ae29553ff _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN5media26VaapiJpegEncodeAccelerator7EncoderEFvNSt3__110unique_ptrINS5_13EncodeRequestENS7_14default_deleteIS9_EEEEEJPS6_SC_EEEvOT_DpOT0_ #7 0x5a1ae295529d _ZN4base8internal7InvokerINS0_9BindStateIMN5media26VaapiJpegEncodeAccelerator7EncoderEFvNSt3__110unique_ptrINS4_13EncodeRequestENS6_14default_deleteIS8_EEEEEJNS0_17UnretainedWrapperIS5_EENS0_13PassedWrapperISB_EEEEEFvvEE7RunImplIRKSD_RKNS6_5tupleIJSF_SH_EEEJLm0ELm1EEEEvOT_OT0_NS6_16integer_sequenceImJXspT1_EEEE #8 0x5a1ae295516c _ZN4base8internal7InvokerINS0_9BindStateIMN5media26VaapiJpegEncodeAccelerator7EncoderEFvNSt3__110unique_ptrINS4_13EncodeRequestENS6_14default_deleteIS8_EEEEEJNS0_17UnretainedWrapperIS5_EENS0_13PassedWrapperISB_EEEEEFvvEE3RunEPNS0_13BindStateBaseE #9 0x5a1ae0aa6f5c _ZNO4base12OnceCallbackIFvvEE3RunEv #10 0x5a1ae30146e8 base::debug::TaskAnnotator::RunTask() #11 0x5a1ae2eee396 base::internal::TaskTracker::RunOrSkipTask() #12 0x5a1ae2fe14cc base::internal::TaskTrackerPosix::RunOrSkipTask() #13 0x5a1ae30bb091 base::test::ScopedTaskEnvironment::TestTaskTracker::RunOrSkipTask() #14 0x5a1ae2eebcd8 base::internal::TaskTracker::RunAndPopNextTask() #15 0x5a1ae30981d5 base::internal::SchedulerWorker::RunWorker() #16 0x5a1ae3097939 base::internal::SchedulerWorker::RunSharedWorker() #17 0x5a1ae309776d base::internal::SchedulerWorker::ThreadMain() #18 0x5a1ae2fe2b06 base::(anonymous namespace)::ThreadFunc() #19 0x7be637ee24fe <unknown> #20 0x7be6373f1bef clone Aborted (core dumped) This seems to be caused by the DCHECK(va_surface_ids_.empty()) check failing in VaapiWrapper::CreateSurfaces.
,
Nov 16
How is picture taking done in ARC++? Is a single jpeg encoder re-used for each picture taken? In that case we should cherry-pick this fix to stable quickly.
,
Nov 16
Yes, the same encoder instance is re-used for each picture taken. We should fix this. Shik recently caught memory-leak in his fuzzer test with JPEG encoder, and perhaps this is the root cause.
,
Nov 16
,
Nov 16
I'm pretty sure this is the root cause! Each picture taken will leak a few MB, so it ramps up quickly.
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cddb472b0a55132ee348f48bf2e29f65a7f4358 commit 5cddb472b0a55132ee348f48bf2e29f65a7f4358 Author: David Staessens <dstaessens@chromium.org> Date: Fri Nov 16 06:50:07 2018 media/gpu/vaapi: Reuse surface if size doesn't change between jpeg encodes. When decoding a jpeg the generated surface is reused when the video frame size doesn't change. When encoding a jpeg a new surface is created every time. This change enables reusing of surfaces for jpeg encoding. This also fixes a bug that causes the jpeg_encode_accelerator_unittest to always fail in debug, because surfaces are not properly destroyed before creating new ones. TEST=JPEG encode tests on nocturne BUG= 905502 Change-Id: I9a6b0b5703813e518a3e82687b289e94745e1c62 Reviewed-on: https://chromium-review.googlesource.com/c/1337130 Commit-Queue: David Staessens <dstaessens@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Cr-Commit-Position: refs/heads/master@{#608683} [modify] https://crrev.com/5cddb472b0a55132ee348f48bf2e29f65a7f4358/media/gpu/vaapi/vaapi_jpeg_encode_accelerator.cc [modify] https://crrev.com/5cddb472b0a55132ee348f48bf2e29f65a7f4358/media/gpu/vaapi/vaapi_wrapper.cc
,
Nov 16
,
Nov 16
Pls apply appropriate OSs label.
,
Nov 16
,
Nov 17
The bug is marked as P3 or Feature. It should not be merged as M71 is in beta. Please contact the approriate milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 19
,
Nov 19
Fixing the memory leak that could eventually crash the system is certainly not P3. :)
,
Nov 19
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review Please contact the milestone owner if you have questions. Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 19
Approved for M71 ChromeOS
,
Nov 22
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Nov 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49c42cf587713ad5c74169e81419cb64db26e9b3 commit 49c42cf587713ad5c74169e81419cb64db26e9b3 Author: David Staessens <dstaessens@chromium.org> Date: Fri Nov 23 07:28:24 2018 media/gpu/vaapi: Reuse surface if size doesn't change between jpeg encodes. When decoding a jpeg the generated surface is reused when the video frame size doesn't change. When encoding a jpeg a new surface is created every time. This change enables reusing of surfaces for jpeg encoding. This also fixes a bug that causes the jpeg_encode_accelerator_unittest to always fail in debug, because surfaces are not properly destroyed before creating new ones. TEST=JPEG encode tests on nocturne BUG= 905502 Change-Id: I9a6b0b5703813e518a3e82687b289e94745e1c62 Reviewed-on: https://chromium-review.googlesource.com/c/1337130 Commit-Queue: David Staessens <dstaessens@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#608683}(cherry picked from commit 5cddb472b0a55132ee348f48bf2e29f65a7f4358) Reviewed-on: https://chromium-review.googlesource.com/c/1343400 Reviewed-by: Ricky Liang <jcliang@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#800} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/49c42cf587713ad5c74169e81419cb64db26e9b3/media/gpu/vaapi/vaapi_jpeg_encode_accelerator.cc [modify] https://crrev.com/49c42cf587713ad5c74169e81419cb64db26e9b3/media/gpu/vaapi/vaapi_wrapper.cc
,
Nov 23
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/49c42cf587713ad5c74169e81419cb64db26e9b3 Commit: 49c42cf587713ad5c74169e81419cb64db26e9b3 Author: dstaessens@chromium.org Commiter: jcliang@chromium.org Date: 2018-11-23 07:28:24 +0000 UTC media/gpu/vaapi: Reuse surface if size doesn't change between jpeg encodes. When decoding a jpeg the generated surface is reused when the video frame size doesn't change. When encoding a jpeg a new surface is created every time. This change enables reusing of surfaces for jpeg encoding. This also fixes a bug that causes the jpeg_encode_accelerator_unittest to always fail in debug, because surfaces are not properly destroyed before creating new ones. TEST=JPEG encode tests on nocturne BUG= 905502 Change-Id: I9a6b0b5703813e518a3e82687b289e94745e1c62 Reviewed-on: https://chromium-review.googlesource.com/c/1337130 Commit-Queue: David Staessens <dstaessens@chromium.org> Reviewed-by: Pawel Osciak <posciak@chromium.org> Reviewed-by: Hirokazu Honda <hiroh@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#608683}(cherry picked from commit 5cddb472b0a55132ee348f48bf2e29f65a7f4358) Reviewed-on: https://chromium-review.googlesource.com/c/1343400 Reviewed-by: Ricky Liang <jcliang@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#800} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
,
Nov 26
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by dstaessens@google.com
, Nov 16