Issue metadata
Sign in to add a comment
|
41.3%-50% regression in browser_tests at 442070:442081 |
||||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jan 17 2017
Large change in unique frames and max repeated on mac. The change looks quite similar to https://bugs.chromium.org/p/chromium/issues/detail?id=681815, but it is associated with a commit position some 150 units earlier, apparently with no video or webrtc related changes.
,
Jan 17 2017
,
Jan 17 2017
,
Jan 17 2017
I ran the test manually on my Linux dev box and found that the drop starts with commit https://chromium.googlesource.com/chromium/src/+/50252de90c71b7d5d12c30121a82d4e790f74252, which is "WebmMuxer: do not set DefaultDuration if timestamp delta is not near constant". On this revision the test outputs RESULT Unique_frames_count: 720p_VP8= 83 score On the revision before it outputs RESULT Unique_frames_count: 720p_VP8= 141 score mcasas@: Could you have a look at this, please? Also issue 681815, which is most likely related.
,
Jan 17 2017
Issue 681815 has been merged into this issue.
,
Jan 17 2017
IIRC the video/audio quality browser tests use MediaRecorder as a way to record the output of a PeerConnection for quality post comparison. The mentioned CL was needed to make MSE work nicely with MR, and should not have affected external players such as e.g. VLC. So I'm tempted to mark this as a WontFix and let the bots gain their new ground. However, I'd like to ask phoglund@ and/or ehmaldonado@ since they wrote the Webm postprocessing functions in [1,2]. [1] https://cs.chromium.org/chromium/src/chrome/browser/media/webrtc/webrtc_video_quality_browsertest.cc?q=WriteCapturedWebmVideo&sq=package:chromium&l=131&dr=CSs [2] https://cs.chromium.org/chromium/src/chrome/browser/media/webrtc/webrtc_video_quality_browsertest.cc?q=RunWebmToI420Converter&sq=package:chromium&l=145&dr=CSs
,
Jan 18 2017
Those functions write the Webm video to a file, and then convert it to I420 using ffmpeg. I'm not sure if that would affect the output that ffmpeg produces.
,
Jan 20 2017
Since we have a major regression on unique frames (much fewer) it tells me performance has gone a lot worse - we simply cannot keep up the pace, which is serious. The fact that max_repeated frames drops is normally a good thing, but likely cause simply by the fact that fewer frames are able to make it through the test. Can we just revert https://chromium.googlesource.com/chromium/src/+/50252de90c71b7d5d12c30121a82d4e790f74252 and see if the graphs recover?
,
Jan 20 2017
I would strongly advise against reverting: the CL was legitimate and solved a real problem when playing back the video, in Chrome. VLC is still playing fine the same video, so in this case I'd say our test infra is not working correctly.
,
Jan 20 2017
,
Jan 23 2017
Assuming Miguel has no plans to isolate the culprit, I'm reassigning to Edward. Can you run the test (Linux is fine, since it regresses on all platforms) and see if you can see what's causing the performance slowdown? Locally reverting the mentioned CL could also be interesting, to see if it solves everything.
,
Feb 22 2017
,
Feb 22 2017
,
Mar 14 2017
Edward - is there something left to be done for this issue?
,
Mar 14 2017
I still haven't got around to do what Henrik suggested in #12
,
Mar 14 2017
I reverted that CL locally, and it didn't have any impact.
,
Mar 14 2017
No, sorry, I was mistaken. Reverting it locally does solve everything.
,
Mar 14 2017
Miguel: Sorry for bringing this up again so late. Do you think this regression is a WebRTC regression?
,
Mar 15 2017
#19: I think this is an issue with the test set up, most likely with the ffmpeg scripting used to extract the individual frames out of the recording. We should start digging there. Try to get the recorded video (start by finding the ondataavailable JS event listener).
,
Mar 15 2017
Re #20: What makes you think that? These tests have been running stable for years and the tooling part dealing with ffmpeg has never been the cause of any flakiness we've seen. The behavior of the test clearly changed in https://chromium.googlesource.com/chromium/src/+/50252de90c71b7d5d12c30121a82d4e790f74252 but why would that have to do with ffmpeg? Does that tiny code change alter the timing of which frames are recorded into the .webm file or something like that? Then I'd like to understand how you can claim that is not a regression that should be reverted, especially as a revert makes our test go back to the much better performance numbers.
,
Mar 15 2017
Re #21: Does that tiny code change alter the timing of which frames are recorded into the .webm file or something like that? It does look like it (from crbug.com/606000 ): > #22: > The issue is a discontinuity as Matt said. The video frame at time 1.601 is > marked as having duration .033. The next video frame is at time 1.68. > > 1.68 - 1.601 = 0.079 > > So either the time of 1.68 is wrong or the duration of .033 is wrong. The > time is read from the timecode field of this block. The block does not set an > explicit duration, so we look for a default duration. In this case the video > track has a default duration of .033. > > If the track did not have a default duration, we would have set the duration > to be the difference in timestamps between current and next buffer. I've > verified that if the duration is set this way (commenting out reading of > default duration in chrome), the video plays back fine. > #31: > I think this should be a pretty easy fix - simply stop setting track DefaultDuration. Hence the change. So this makes me believe this is an issue that only affects the test toolchain for some reason, and doesn't imply a regression in WebRTC. Miguel: Would you like to have access to some recorded .webm video?
,
Mar 27 2017
Correcting the component. (Blink>WebRTC>Tools is not for infra/test issues.)
,
Apr 21 2017
ehmaldonado@ are you currently looking at this?
,
Apr 21 2017
I'm tempted to close this as Won't Fix. kjellander: What do you think?
,
Apr 21 2017
I don't want to close this just blaming "something is wrong with the toolchain". I'd like to see proof on how that can be and an explanation on why we would suddenly get only half of the frames stored onto disk. The graph at https://chromeperf.appspot.com/report?sid=c5eba42b2ce75fca5a42ff52776e623756ef164c3adfae970d86d6fb2a95afb2&rev=442081 makes this very clear. mcasas: in #20 you're speculating this would have something to do with how we use ffmpeg, can you elaborate? The commands we use are printed in clear in the test logs (and in the code) if you're interested.
,
Apr 21 2017
,
May 23 2017
mcasas@ are you looking at this?
,
Jun 20 2017
Ping.
,
Sep 5 2017
Removing myself as owner of these issues are marking them for retriaging.
,
Sep 5 2017
,
Sep 6 2017
+phoglund Patrik, now that you're back: what's your view on this? We still have regressions of * ~50% for Max_repeated * ~42% for Unique_frames_count since January 17 when this and bug 681815 were filed. It affects all platforms, all codecs and both 360p and 720p. Certainly we can accept the regression if the discussion above is concluding that it doesn't affect the end-user experience. We're likely able to spot new regressions also with the new (lower) baseline. It just feels wrong to ignore what's going on here.
,
Sep 6 2017
I guess we either get less unique frames to the <video> tag, or less unique frames in file produced by the media recorder. That sounds like a clear reduction in performance for the media recorder or maybe even for WebRTC calls in general. Surely it must be better to capture more frames for the same duration of video? I doubt we would a 40% reduction in unique frames would have slipped into major releases unnoticed, but I suppose MediaRecorder could have become worse since it sees less usage. What's a good way to find out, blind tests of recorded videos before and after the CL? Miguel, how did you verify your CL was good and didn't worsen quality?
,
Sep 6 2017
#33: content_browsertests [1] did not regress, and we didn't see any bug reports from the field, where it's being used actively, see the UMA https://uma.googleplex.com/timeline_v2?sid=dfa61b867672d32c08e1b8a93268f50c) [1] https://cs.chromium.org/chromium/src/content/browser/webrtc/webrtc_media_recorder_browsertest.cc?sq=package:chromium&dr=CSs
,
Sep 7 2017
Ok, if you are convinced the media recorder is fine, there's not a lot more to do and we should WontFix this. The regression makes the tests less sensitive, which is unfortunate, but it's not a big deal. The absolute numbers are not that important, only the changes in them. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by nisse@chromium.org
, Jan 17 2017