Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2375153003:
Update threshold for RTCVideoEncoder SW fallback (Closed)
|
Created:
4 years, 2 months ago by emircan Modified:
4 years, 2 months ago CC:
chromium-reviews, mlamouri+watch-content_chromium.org, posciak+watch_chromium.org, jam, feature-media-reviews_chromium.org, darin-cc_chromium.org, piman+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionUpdate threshold for RTCVideoEncoder SW fallback
Earlier on, I added dynamic error tracking for the special issue
explained on crbug.com/594161. We tried to reset HW decoder to
handle H264 errors since SW fallback wasn't available.
Since SW H264 is available now, this issue is no longer applicable.
We should have less tolerance for error and fall back to SW.
BUG=651489, webrtc:6377
TEST=Tested AppRTC loopback H264 decode on Mac.
Committed: https://crrev.com/ed96795836993e079ff0de7cae4ef242894d4057
Cr-Commit-Position: refs/heads/master@{#421847}
Patch Set 1 : Revert crrev.com/1817573004 #Patch Set 2 : Add SW fallback. #Patch Set 3 : Change threshold. #Messages
Total messages: 29 (20 generated)
Description was changed from ========== revert dynamic ahndling BUG= ========== to ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=6377 TEST=Tested appRTc loopback H264 decode on Mac. ==========
emircan@chromium.org changed reviewers: + niklase@chromium.org, posciak@chromium.org, wuchengli@chromium.org
The CQ bit was checked by emircan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL. I decided to clean up this logic as SW H264 is now available and the original issue crbug.com/594161 isn't applicable. AFAIK, HW decoder does not recover after reset for most cases, and resetting only delays playback as seen on https://bugs.chromium.org/p/webrtc/issues/detail?id=6377 and crbug.com/651220. Let me know if you think it is still worth trying a reset, and I can tweak |kNumVDAErrorsBeforeSWFallback| per platform.
The CQ bit was checked by emircan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #2 (id:20001) has been deleted
Description was changed from ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=6377 TEST=Tested appRTc loopback H264 decode on Mac. ========== to ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested appRTc loopback H264 decode on Mac. ==========
Description was changed from ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested appRTc loopback H264 decode on Mac. ========== to ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW encoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ========== to ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ==========
Hmm. I'm worried this will hide the bugs of HW decoders. It looks like Hangout works but the power consumption or CPU increases. On the other hand, I understand not all hardware decode are as robust as software decode.
On 2016/09/29 00:11:25, emircan wrote: > PTAL. > > I decided to clean up this logic as SW H264 is now available and the original > issue crbug.com/594161 isn't applicable. AFAIK, HW decoder does not recover > after reset for most cases, and resetting only delays playback as seen on > https://bugs.chromium.org/p/webrtc/issues/detail?id=6377 and crbug.com/651220. I'm not sure about non-CrOS platforms. For CrOS, reinit VDA should work. Users won't file bugs when resetting VDA works. So seeing crbug.com/61220 doesn't mean HW decoder does not recover after reset for most cases. The fallback probably already hid several hardware decoder bugs. > > Let me know if you think it is still worth trying a reset, and I can tweak > |kNumVDAErrorsBeforeSWFallback| per platform. I think ChromeOS can just use 5 like Android. 50 times is too much.
On 2016/09/29 15:52:01, wuchengli wrote: > > Let me know if you think it is still worth trying a reset, and I can tweak > > |kNumVDAErrorsBeforeSWFallback| per platform. > I think ChromeOS can just use 5 like Android. 50 times is too much. Sounds good then. I will change it to 5 for all platforms. We choose 50 in the beginning as HW H264 was having consecutive errors and didn't have SW fallback. Now that, I have seen Windows HW H264 do not recover with reset ever, there is no point waiting for 50 frames there either.
The CQ bit was checked by emircan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
wuchengli@ PTAL. I just uploaded a patch changing the threshold. Also, PTAL at how reset is done as you didn't have time during crrev.com/1817573004.
LGTM. Please also update the subject and change description.
Description was changed from ========== Revert dynamic error tracking in RTCVideoDecoder Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We can fallback to SW immediately when an error happens. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ========== to ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ==========
Description was changed from ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ========== to ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=651489, webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ==========
The CQ bit was unchecked by emircan@chromium.org
The CQ bit was checked by emircan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=651489, webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ========== to ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=651489, webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ==========
Message was sent while issue was closed.
Committed patchset #3 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=651489, webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. ========== to ========== Update threshold for RTCVideoEncoder SW fallback Earlier on, I added dynamic error tracking for the special issue explained on crbug.com/594161. We tried to reset HW decoder to handle H264 errors since SW fallback wasn't available. Since SW H264 is available now, this issue is no longer applicable. We should have less tolerance for error and fall back to SW. BUG=651489, webrtc:6377 TEST=Tested AppRTC loopback H264 decode on Mac. Committed: https://crrev.com/ed96795836993e079ff0de7cae4ef242894d4057 Cr-Commit-Position: refs/heads/master@{#421847} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/ed96795836993e079ff0de7cae4ef242894d4057 Cr-Commit-Position: refs/heads/master@{#421847} |
