RTCVideoDecoder calls Decoded on different thread than Decode was called on |
|||
Issue descriptionWhat steps will reproduce the problem? (1) Patch this CL to Chromium https://codereview.webrtc.org/2811963004/ (2) Run "./out/AndroidDebug/bin/run_content_browsertests --fast-local-dev --gtest-filter="*WebRtcBrowserTest.CanSetupAudioAndVideoCall*" What is the expected result? Test passes What happens instead? Test fails 04-18 11:31:10.686 5391 5462 E rtc : # Fatal error in ../../third_party/webrtc/modules/video_coding/generic_decoder.cc, line 67 04-18 11:31:10.686 5391 5462 E rtc : # last system error: 30 04-18 11:31:10.686 5391 5462 E rtc : # Check failed: rtc::internal::AnnounceOnThread::IsCurrent(&decoder_thread_) This problem affects at least Android. It might affect affect other platforms as well. Full stack trace: ********** Crash dump: ********** Build fingerprint: 'motorola/athene_f/athene_f:7.0/NPJ25.93-14/16:user/release-keys' pid: 5869, tid: 5953, name: Media >>> org.chromium.content_browsertests_apk:sandboxed_process0 <<< signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr -------- Stack frame #00 pc 00049bc4 /system/lib/libc.so (tgkill+12) Stack frame #01 pc 00047363 /system/lib/libc.so (pthread_kill+34) Stack frame #02 pc 0001d535 /system/lib/libc.so (raise+10) Stack frame #03 pc 00019081 /system/lib/libc.so (__libc_android_abort+34) Stack frame #04 pc 000170e4 /system/lib/libc.so (abort+4) Stack frame #05 pc 00c73a1d /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so: Routine rtc::FatalMessage::~FatalMessage() at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../third_party/webrtc/base/checks.cc:110 Stack frame #06 pc 00fbfafb /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so: Routine webrtc::VCMDecodedFrameCallback::Decoded(webrtc::VideoFrame&, rtc::Optional<int>, rtc::Optional<unsigned char>) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../third_party/webrtc/modules/video_coding/generic_decoder.cc:68 (discriminator 13) Stack frame #07 pc 00fbf9c9 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so: Routine webrtc::VCMDecodedFrameCallback::Decoded(webrtc::VideoFrame&, long long) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../third_party/webrtc/modules/video_coding/generic_decoder.cc:60 (discriminator 7) Stack frame #08 pc 00ba7167 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so (_ZN7content15RTCVideoDecoder12PictureReadyERKN5media7PictureE+730): Routine content::RTCVideoDecoder::PictureReady(media::Picture const&) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../content/renderer/media/gpu/rtc_video_decoder.cc:434 Stack frame #09 pc 006e2a91 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so: Routine media::GpuVideoDecodeAcceleratorHost::OnPictureReady(AcceleratedVideoDecoderHostMsg_PictureReady_Params const&) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../media/gpu/ipc/client/gpu_video_decode_accelerator_host.cc:268 Stack frame #10 pc 006e38cb /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libcontent.cr.so: Routine void base::DispatchToMethodImpl<media::GpuVideoDecodeAcceleratorHost*, void (media::GpuVideoDecodeAcceleratorHost::*)(AcceleratedVideoDecoderHostMsg_PictureReady_Params const&), std::__ndk1::tuple<AcceleratedVideoDecoderHostMsg_PictureReady_Params> const&, 0u>(media::GpuVideoDecodeAcceleratorHost* const&, void (media::GpuVideoDecodeAcceleratorHost::*)(AcceleratedVideoDecoderHostMsg_PictureReady_Params const&), std::__ndk1::tuple<AcceleratedVideoDecoderHostMsg_PictureReady_Params> const&, base::IndexSequence<0u>) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/tuple.h:77 (discriminator 6) Stack frame #11 pc 00091e37 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base5debug13TaskAnnotator7RunTaskEPKcPNS_11PendingTaskE+598): Routine base::Callback<void (), (base::internal::CopyMode)0, (base::internal::RepeatMode)0>::Run() && at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/callback.h:91 (discriminator 1) Stack frame #12 pc 000ae1f3 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base11MessageLoop7RunTaskEPNS_11PendingTaskE+442): Routine base::MessageLoop::RunTask(base::PendingTask*) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/message_loop/message_loop.cc:423 Stack frame #13 pc 000ae659 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE+28): Routine base::MessageLoop::DeferOrRunPendingTask(base::PendingTask) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/message_loop/message_loop.cc:434 Stack frame #14 pc 000ae743 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base11MessageLoop6DoWorkEv+144): Routine base::MessageLoop::DoWork() at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/message_loop/message_loop.cc:527 (discriminator 2) Stack frame #15 pc 000b0e5f /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base18MessagePumpDefault3RunEPNS_11MessagePump8DelegateE+90): Routine base::MessagePumpDefault::Run(base::MessagePump::Delegate*) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/message_loop/message_pump_default.cc:33 Stack frame #16 pc 000afcb9 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base11MessageLoop10RunHandlerEv+168): Routine base::MessageLoop::RunHandler() at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/message_loop/message_loop.cc:387 (discriminator 1) Stack frame #17 pc 000caa07 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base7RunLoop3RunEv+86): Routine base::RunLoop::Run() at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/run_loop.cc:37 Stack frame #18 pc 000ed9ef /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base6Thread3RunEPNS_7RunLoopE+126): Routine base::Thread::Run(base::RunLoop*) at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/threading/thread.cc:250 Stack frame #19 pc 000ee72b /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so (_ZN4base6Thread10ThreadMainEv+686): Routine base::Thread::ThreadMain() at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/threading/thread.cc:333 Stack frame #20 pc 000e8dd5 /data/app/org.chromium.content_browsertests_apk-1/lib/arm/libbase.cr.so: Routine ThreadFunc at /usr/local/google/home/sakal/code/chromium/src/out/AndroidDebug/../../base/threading/platform_thread_posix.cc:71 Stack frame #21 pc 00046e33 /system/lib/libc.so (_ZL15__pthread_startPv+22) Stack frame #22 pc 00019acd /system/lib/libc.so (__start_thread+6) Crash dump is completed
,
Apr 18 2017
That would work, but I am not sure about adding another data structure and delaying the result until next Decode() call. We can just try to post Decoded() calls on the thread Decode() is called. i.e., can we save the target thread by using base::ThreadTaskRunnerHandle::Get() on Decode() call or use media::BindToCurrentLoop()? I am not sure if those would work on rtc's platform threads. Alternatively, can we add a wrapper around webrtc::DecodedImageCallback such that it posts the Decoded() calls on the right thread?
,
Apr 19 2017
I don't think these methods will work on rtc::PlatformThread. Sadly, I don't think it is possible to post on rtc::PlatformThread at all and that is why we have this polling as a work around.
,
Apr 19 2017
It's critical to call decoded image callback as soon as possible. We must not add latency to real time video decoding (at least on ChromeOS). We tried hard to optimize the hardware decoding latency. Every millisecond counts. Here are the options: - Call Decoded until next Decode: this adds too much latency. The latency can increase 33ms in the worse case. - Make Decode synchronous: That is, Decode returns until Decoded is called. But not every Decode generates a decoded frame for H264. From VDA API, it's not guaranteed to have a PictureReady for every Decode. - PollDecodedFrames: Does this mean adding RtcVideoDecoder::PollDecodedFrames and polling every 10ms? This doesn't work for ChromeOS. 10ms is too much. Reducing the interval increased CPU usage. I think we should either make webrtc thread postable or allow a different thread that calls Decoded.
,
Apr 20 2017
We will replace the rtc::PlatformThread with TaskQueue eventually. However, that will take some time. We would like to land this CL before that. If PollDecodedFrames is not acceptable even as a temporary solution, then we would have to look into making Decode synchronous. I am not sure if just invoking RequestBufferDecode instead of posting it would be enough? Then we could empty the frame queue after calling it.
,
Apr 20 2017
Thanks Sami. In fact the CL we want to land is a part of preparing the decoding pipeline to be able to use the TaskQueue class. WebRTC's TaskQueue class is compatible with Chrome's SingleThreadTaskRunner btw.
,
Apr 20 2017
Re #5: Invoking or posting RequestBufferDecode is not the main problem. The problem is RtcVideoDecoder doesn't know if VDA::Decode will generate a decoded frame. So RtcVideoDecoder::Decode cannot wait for VDA::Client::PictureReady.
,
Apr 20 2017
Instead of polling, which is a temporary hack, would it be better if we changed the hack to support an rtc::Event instead of polling at an interval? I.e. the decoder (Android only) would have the ability to give signal an event that breaks the wait/sleep that runs on the decoder thread. When that event gets signaled, the poll method gets called, otherwise it doesn't get called.
,
Apr 20 2017
"ability to give signal an event" -> "ability to signal an event" I'm basically thinking of a poor man's PostTask() until we get the real thing.
,
Apr 25 2017
So is this event based signaling solution something we can agree on?
,
Apr 25 2017
Event based solution sounds good to me as a temp fix. We should keep an eye on low-end CrOS devices in case of a regression. Adding the functionality to post tasks on WebRTC would be an awesome improvement btw. It came up as a limitation many times, see Limitation#5 on https://goo.gl/se8fH9 for instance.
,
Apr 26 2017
This threading problem is not only on Android. RtcVideoDecoder sends Decoded callback on media thread. So the solution should work on all platforms. Also, let's not add more #ifdef ANDROID in RtcVideoDecoder. The existing #ifdef ANDROID caused me some headaches last time. If I understand correctly, event based solution works like this. - Add a VideoDecoder::RegisterDecodedEventSingaling(rtc::Event) and VideoDecoder::PollDecodedFrames. - WebRTC registers a rtc::Event in RtcVideoDecoder. - When there's a decoded frame, RtcVideoDecoder calls event_->Set(). WebRTC gets the notification and calls RtcVideoDecoder::PollDecodedFrames. RtcVideoDecoder calls DecodedImageCallback in the function. This should work but isn't it the same as sending the decoded frame in a different thread because you still need synchronization between media thread and the thread that calls Decode? Another way to do event based solution is to move rtc::Event to DecodedImageCallback of WebRTC. After event is signaled in DecodedImageCallback, you can use the thread you want to get the decoded frame. RtcVideoDecoder doesn't need to change at all.
,
May 23 2017
,
May 23 2017
|
|||
►
Sign in to add a comment |
|||
Comment 1 by sakal@chromium.org
, Apr 18 2017