Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6)

Issue 2792033003: Revert of Deliver video frames on Android, on the decode thread. (Closed)

Created:
3 years, 8 months ago by guidou
Modified:
3 years, 8 months ago
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Revert of Deliver video frames on Android, on the decode thread. (patchset #7 id:120001 of https://codereview.webrtc.org/2764573002/ ) Reason for revert: Breaks Chrome FYI Android bots. See: https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28L%20Nexus9%29/builds/20418 https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28L%20Nexus6%29/builds/14724 https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28L%20Nexus5%29/builds/20133 https://build.chromium.org/p/chromium.webrtc.fyi/builders/Android%20Tests%20%28dbg%29%20%28K%20Nexus5%29/builds/15111 Original issue's description: > Deliver video frames on Android, on the decode thread. > > VideoCoding > * Adding a method for polling for frames on Android only until the capture implementation takes care of this (longer term plan). > > CodecDatabase > * Add an accessor for the current decoder > * Use std::unique_ptr<> for ownership. > * Remove "Release()" and "ReleaseDecoder()". Instead just delete. > * Remove |friend| relationship between CodecDatabase and VCMGenericDecoder. > > VCMDecodedFrameCallback > * DCHECKs for thread correctness. > * Remove |lock_| now that a threading model has been established and verified. > > VCMGenericDecoder > * All methods now have thread checks. > * Variable access associated with thread checkers. > > VideoReceiver > * Added two notification methods, DecoderThreadStarting() and DecoderThreadStopped() > * Allows us to establish a period when the decoder thread is not running and it is safe to modify variables such as callbacks, that are only read when the decoder thread is running. > * Allows us to DCHECK thread guarantees. > * Allows synchronizing callbacks from the module process thread and have them only active while the decoder thread is running. > * The above, allows us to establish two modes for the thread, single-threaded-mutable and multi-threaded-const. > * Using that knowledge, we can remove |receive_crit_| as well as locking for a number of member variables. > > MediaCodecVideoDecoder > * Removed frame polling code from this class, since this is now done from the root thread function in VideoReceiveStream. > > VideoReceiveStream > * On Android: Polls for decoded frames every 10ms (same interval as previously in MediaCodecVideoDecoder) > * [Un]Registers the |video_receiver_| with the module thread only around the time the decoder thread is started/stopped. > * Notifies the receiver of start/stop events of the decoder thread. > * Changed the decoder thread to use the new PlatformThread callback type. > > BUG=webrtc:7361, 695438 > > Review-Url: https://codereview.webrtc.org/2764573002 > Cr-Commit-Position: refs/heads/master@{#17527} > Committed: https://chromium.googlesource.com/external/webrtc/+/e3aa88bbd5accadec73fa7e38584dfbf6aabe8a9 TBR=sakal@webrtc.org,mflodman@webrtc.org,stefan@webrtc.org,tommi@webrtc.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=webrtc:7361, 695438 Review-Url: https://codereview.webrtc.org/2792033003 Cr-Commit-Position: refs/heads/master@{#17530} Committed: https://chromium.googlesource.com/external/webrtc/+/c3372583d4314eff1c6ab4a965b746aca08a06f8

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+332 lines, -522 lines) Patch
M webrtc/media/engine/videodecodersoftwarefallbackwrapper.h View 1 chunk +0 lines, -5 lines 0 comments Download
M webrtc/media/engine/videodecodersoftwarefallbackwrapper.cc View 1 chunk +0 lines, -8 lines 0 comments Download
M webrtc/modules/video_coding/codec_database.h View 3 chunks +9 lines, -7 lines 0 comments Download
M webrtc/modules/video_coding/codec_database.cc View 8 chunks +51 lines, -42 lines 0 comments Download
M webrtc/modules/video_coding/generic_decoder.h View 5 chunks +21 lines, -24 lines 0 comments Download
M webrtc/modules/video_coding/generic_decoder.cc View 5 chunks +50 lines, -67 lines 0 comments Download
M webrtc/modules/video_coding/video_coding_impl.h View 4 chunks +21 lines, -51 lines 0 comments Download
M webrtc/modules/video_coding/video_receiver.cc View 21 chunks +73 lines, -148 lines 0 comments Download
M webrtc/sdk/android/src/jni/androidmediadecoder_jni.cc View 26 chunks +82 lines, -114 lines 0 comments Download
M webrtc/video/video_receive_stream.h View 1 chunk +1 line, -1 line 0 comments Download
M webrtc/video/video_receive_stream.cc View 6 chunks +24 lines, -48 lines 0 comments Download
M webrtc/video_decoder.h View 1 chunk +0 lines, -7 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
guidou
Created Revert of Deliver video frames on Android, on the decode thread.
3 years, 8 months ago (2017-04-04 14:16:02 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/2792033003/1
3 years, 8 months ago (2017-04-04 14:16:13 UTC) #3
commit-bot: I haz the power
Committed patchset #1 (id:1) as https://chromium.googlesource.com/external/webrtc/+/c3372583d4314eff1c6ab4a965b746aca08a06f8
3 years, 8 months ago (2017-04-04 14:16:25 UTC) #6
tommi
3 years, 8 months ago (2017-04-11 12:52:07 UTC) #7
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in
https://codereview.webrtc.org/2811963004/ by tommi@webrtc.org.

The reason for reverting is: Preparing to reland. Will include Sami's changes..

Powered by Google App Engine
This is Rietveld 408576698