DCHECK crash with Picture-in-Picture and MediaStreams |
||||||||||
Issue descriptionChromium : 71.0.3563.0 (Developer build) What steps will reproduce the problem? 1. Go to https://beaufortfrancois.github.io/sandbox/media/picture-in-picture-playground 2. Click "Toggle Picture-in-Picture" button (video.requestPictureInPicture) 3. Click "Reset" button (video.src = null) 4. Click "Toggle MediaStream" button (video.srcObject = mediaStream) What is the expected result? MediaStream video should be visible in PiP window What happens instead of that? Tab crashes. [26592:26592:0927/125847.993989:FATAL:webmediaplayer_ms_compositor.cc(50)] Check failed: frame->format() == media::PIXEL_FORMAT_ARGB || frame->format() == media::PIXEL_FORMAT_XRGB || frame->format() == media::PIXEL_FORMAT_I420 || frame->format() == media::PIXEL_FORMAT_UYVY || frame->format() == media::PIXEL_FORMAT_NV12. #0 0x7fd6c40da1bd base::debug::StackTrace::StackTrace() #1 0x7fd6c3dc0f7c base::debug::StackTrace::StackTrace() #2 0x7fd6c3deccad logging::LogMessage::~LogMessage() #3 0x7fd6be356545 content::(anonymous namespace)::CopyFrame() #4 0x7fd6be355caa content::WebMediaPlayerMSCompositor::ReplaceCurrentFrameWithACopyInternal() #5 0x7fd6bb83548f _ZN4base8internal13FunctorTraitsIMN7content28ServiceManagerConnectionImpl15IOThreadContextEFvvEvE6InvokeIS6_13scoped_refptrIS4_EJEEEvT_OT0_DpOT1_ #6 0x7fd6bc18e434 _ZN4base8internal12InvokeHelperILb0EPN2gl9GLContextEE8MakeItSoIRKMN5media19CommandBufferHelperEFS4_vEJRK13scoped_refptrIS8_EEEES4_OT_DpOT0_ #7 0x7fd6be3a0900 _ZN4base8internal7InvokerINS0_9BindStateIMN7content26WebMediaPlayerMSCompositorEFvvEJ13scoped_refptrIS4_EEEEFvvEE7RunImplIRKS6_RKNSt3__15tupleIJS8_EEEJLm0EEEEvOT_OT0_NSF_16integer_sequenceImJXspT1_EEEE #8 0x7fd6be3a089c _ZN4base8internal7InvokerINS0_9BindStateIMN7content26WebMediaPlayerMSCompositorEFvvEJ13scoped_refptrIS4_EEEEFvvEE3RunEPNS0_13BindStateBaseE #9 0x7fd6bb82774e _ZNO4base17RepeatingCallbackIFvvEE3RunEv #10 0x7fd6bd45c8a6 _ZN4base8internal13FunctorTraitsINS_17RepeatingCallbackIFvvEEEvE6InvokeIS4_JEEEvOT_DpOT0_ #11 0x7fd6bd45c7ed _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoINS_17RepeatingCallbackIFvvEEEJEEEvOT_DpOT0_ #12 0x7fd6bd45c7c1 _ZN4base8internal7InvokerINS0_9BindStateINS_17RepeatingCallbackIFvvEEEJEEES4_E7RunImplIS5_NSt3__15tupleIJEEEJEEEvOT_OT0_NS9_16integer_sequenceImJXspT1_EEEE #13 0x7fd6bd45c749 _ZN4base8internal7InvokerINS0_9BindStateINS_17RepeatingCallbackIFvvEEEJEEES4_E7RunOnceEPNS0_13BindStateBaseE #14 0x7fd6c3e0aefe _ZNO4base12OnceCallbackIFvvEE3RunEv #15 0x7fd6c3dc21ea base::debug::TaskAnnotator::RunTask() #16 0x7fd6c3f72ae2 base::sequence_manager::internal::ThreadControllerImpl::DoWork() #17 0x7fd6c3fd31d1 _ZN4base8internal13FunctorTraitsIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS4_8WorkTypeEEvE6InvokeIS7_RKNS_7WeakPtrIS4_EEJRKS5_EEEvT_OT0_DpOT1_ #18 0x7fd6c3fd3135 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMNS_16sequence_manager8internal20ThreadControllerImplEFvNS6_8WorkTypeEERKNS_7WeakPtrIS6_EEJRKS7_EEEvOT_OT0_DpOT1_ #19 0x7fd6c3fd30ad _ZN4base8internal7InvokerINS0_9BindStateIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS5_8WorkTypeEEJNS_7WeakPtrIS5_EES6_EEEFvvEE7RunImplIRKS8_RKNSt3__15tupleIJSA_S6_EEEJLm0ELm1EEEEvOT_OT0_NSH_16integer_sequenceImJXspT1_EEEE #20 0x7fd6c3fd2fac _ZN4base8internal7InvokerINS0_9BindStateIMNS_16sequence_manager8internal20ThreadControllerImplEFvNS5_8WorkTypeEEJNS_7WeakPtrIS5_EES6_EEEFvvEE3RunEPNS0_13BindStateBaseE #21 0x7fd6c3e0aefe _ZNO4base12OnceCallbackIFvvEE3RunEv #22 0x7fd6c3dc21ea base::debug::TaskAnnotator::RunTask() #23 0x7fd6c3e7ee58 base::MessageLoop::RunTask() #24 0x7fd6c3e7f15b base::MessageLoop::DeferOrRunPendingTask() #25 0x7fd6c3e7f5a4 base::MessageLoop::DoWork() #26 0x7fd6c3e81f18 base::MessagePumpDefault::Run() #27 0x7fd6c3e7e62e base::MessageLoop::Run() #28 0x7fd6c3f228a2 base::RunLoop::Run() #29 0x7fd6be4f5c55 content::RendererMain() #30 0x7fd6be8251e3 content::RunZygote() #31 0x7fd6be827579 content::RunOtherNamedProcessTypeMain() #32 0x7fd6be829971 content::ContentMainRunnerImpl::Run() #33 0x7fd6be81ea9c content::ContentServiceManagerMainDelegate::RunEmbedderProcess() #34 0x7fd6c4403b81 service_manager::Main() #35 0x7fd6be824c15 content::ContentMain() #36 0x5610b6d68346 ChromeMain #37 0x5610b6d68252 main #38 0x7fd695e76830 __libc_start_main #39 0x5610b6d6812a _start
,
Sep 27
,
Oct 18
,
Oct 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9358262870768c862c8e4bf0a6b5515b572d86be commit 9358262870768c862c8e4bf0a6b5515b572d86be Author: CJ DiMeglio <lethalantidote@chromium.org> Date: Mon Oct 22 21:46:57 2018 Allows for on demand switching to surfaces for PiP for MediaStreams This change allows the kOnDemand option to apply for MediaStreams, meaning that we can switch over to using Surfaces when it is needed for PiP, deferring to a VideoLayer in the normal non-PiP case. Bug: 889857 Change-Id: I56019885d24e9d36f74a279d31f8259081edd1d3 Reviewed-on: https://chromium-review.googlesource.com/c/1271591 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Emircan Uysaler <emircan@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: François Beaufort <beaufort.francois@gmail.com> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Cr-Commit-Position: refs/heads/master@{#601738} [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/media_factory.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/media_factory.h [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/stream/webmediaplayer_ms.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/stream/webmediaplayer_ms.h [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/stream/webmediaplayer_ms_compositor.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/stream/webmediaplayer_ms_compositor.h [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/content/renderer/media/stream/webmediaplayer_ms_unittest.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/third_party/blink/renderer/core/frame/picture_in_picture_controller.h [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/third_party/blink/renderer/core/html/media/html_video_element.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/third_party/blink/renderer/core/html/media/html_video_element.h [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/third_party/blink/renderer/modules/picture_in_picture/html_video_element_picture_in_picture.cc [modify] https://crrev.com/9358262870768c862c8e4bf0a6b5515b572d86be/third_party/blink/renderer/modules/picture_in_picture/picture_in_picture_controller_impl.cc
,
Oct 22
,
Oct 22
Will ask for merge on Wednesday after it has shown to be good on dev/canary.
,
Oct 24
71 Merge Request for https://chromium-review.googlesource.com/c/1271591. This fixes a DCHECK crash for Picture-in-Picture. It is a safe change.
,
Oct 25
This change impacts a number of areas. Can you add context regarding testing on ToT/M72 or otherwise prior to a merge? I'd like to understand the merge risk. I know you're saying it's safe, but still a lot of areas when we're trying to get stable. Thanks
,
Oct 25
This should only effect surface layer for video, which is currently disabled behind a flag, so it should have no effect on the stability of the browser.
,
Oct 25
The change is touching a lot of code because some stuff needed to be plumbed through. The change will allow us to experiment with the feature in Beta. The intent is to be able to use a finch experiment.
,
Oct 25
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
,
Oct 25
Approving merge to M71 Chrome OS.
,
Oct 26
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6 commit c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6 Author: Mounir Lamouri <mlamouri@chromium.org> Date: Tue Oct 30 16:24:43 2018 Allows for on demand switching to surfaces for PiP for MediaStreams This change allows the kOnDemand option to apply for MediaStreams, meaning that we can switch over to using Surfaces when it is needed for PiP, deferring to a VideoLayer in the normal non-PiP case. TBR=lethalantidote@chromium.org (cherry picked from commit 9358262870768c862c8e4bf0a6b5515b572d86be) Bug: 889857 Change-Id: I56019885d24e9d36f74a279d31f8259081edd1d3 Reviewed-on: https://chromium-review.googlesource.com/c/1271591 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Emircan Uysaler <emircan@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: François Beaufort <beaufort.francois@gmail.com> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601738} Reviewed-on: https://chromium-review.googlesource.com/c/1307897 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#397} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/media_factory.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/media_factory.h [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/stream/webmediaplayer_ms.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/stream/webmediaplayer_ms.h [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/stream/webmediaplayer_ms_compositor.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/stream/webmediaplayer_ms_compositor.h [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/content/renderer/media/stream/webmediaplayer_ms_unittest.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/third_party/blink/renderer/core/frame/picture_in_picture_controller.h [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/third_party/blink/renderer/core/html/media/html_video_element.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/third_party/blink/renderer/core/html/media/html_video_element.h [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/third_party/blink/renderer/modules/picture_in_picture/html_video_element_picture_in_picture.cc [modify] https://crrev.com/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6/third_party/blink/renderer/modules/picture_in_picture/picture_in_picture_controller_impl.cc
,
Oct 30
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
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6 Commit: c5fd4a9b9913d6b74f2fb799b70518e16c2de6f6 Author: mlamouri@chromium.org Commiter: mlamouri@chromium.org Date: 2018-10-30 16:24:43 +0000 UTC Allows for on demand switching to surfaces for PiP for MediaStreams This change allows the kOnDemand option to apply for MediaStreams, meaning that we can switch over to using Surfaces when it is needed for PiP, deferring to a VideoLayer in the normal non-PiP case. TBR=lethalantidote@chromium.org (cherry picked from commit 9358262870768c862c8e4bf0a6b5515b572d86be) Bug: 889857 Change-Id: I56019885d24e9d36f74a279d31f8259081edd1d3 Reviewed-on: https://chromium-review.googlesource.com/c/1271591 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Emircan Uysaler <emircan@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: François Beaufort <beaufort.francois@gmail.com> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#601738} Reviewed-on: https://chromium-review.googlesource.com/c/1307897 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#397} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by mlamouri@chromium.org
, Sep 27