New issue
Advanced search Search tips

Issue 663709 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Flaky video frame hashing

Project Member Reported by wdzierza...@opera.com, Nov 9 2016

Issue description

Version: 56.0.2896.3

AFAICS the current video frame hashing implementation in PipelineIntegrationTestBase is not 100% deterministic.  I have a test MP4 file with 44.1 kHz audio and 82 frames of 29.97 fps video.  There are two hashing results possible:

  - sometimes VideoRendererImpl will fire the "ended" callback from Render() after rendering 82 frames,
  - but more often than not it will fire it after 81 frames (when it gets the EOS frame and effective_frames_queued is 0).

There are 82 "ready" frames altogether in both cases.

Unfortunately, there's not much use in me publishing a test case, because I've only seen this happen with Opera's own set of audio and video decoders.  But I can provide more symptoms (logs, etc.) if needed.  So far I've noticed the audio and video timestamps produced by these decoders are close to, but not exactly the same as those produced by FFmpeg.

For reasons not entirely clear to me, I've only started seeing this problem after https://codereview.chromium.org/2375713003/.  Since the audio renderer doubles as the time source for the video renderer, that CL must be affecting the audio renderer in a way that triggers this instability.  (The video hashes are consistently computed over 82 frames again if I remove the audio track.)
 
effective_frames_queued() likely shouldn't be 0 if only 81 frames have been rendered; so that sounds like this is a bug. The frame should only be counted as ineffective if it's been rendered or the clock has advanced so far that the 82nd frame is too far in the past.

In testing mode we force frame_dropping=false, so we return every frame in order regardless of smoothing. What might be happening is effective frames queued needs to compensate for when frame dropping is disabled.

Is this running on a pretty slow machine?
Not particularly slow, an i7-4770 @ 3.40 GHz.

Also, interestingly, 81 is the most common case, on this and several other machines.  It's actually quite hard to get 82, and enabling verbose logging or exerting some load on the CPU seems to help.

Attaching logs from the two test outcomes.  Let me know if more debug info would be useful.
test-81.log
258 KB View Download
test-82.log
256 KB View Download
You might try changing VideoRendererAlgorithm::UpdateEffectiveFramesQueued() to just return frame_queue_.size() when frame_dropping_disabled_ == true. It's not 100% accurate then but it should reveal if that's the issue here. 
I tried

diff --git a/media/filters/video_renderer_algorithm.cc b/media/filters/video_renderer_algorithm.cc
index 8bd7c5c..97d8954 100644
--- a/media/filters/video_renderer_algorithm.cc
+++ b/media/filters/video_renderer_algorithm.cc
@@ -691,7 +691,7 @@ base::TimeDelta VideoRendererAlgorithm::CalculateAbsoluteDriftForFrame(

 void VideoRendererAlgorithm::UpdateEffectiveFramesQueued() {
   if (frame_queue_.empty() || average_frame_duration_.is_zero() ||
-      last_deadline_max_.is_null()) {
+      last_deadline_max_.is_null() || frame_dropping_disabled_) {
     effective_frames_queued_ = frame_queue_.size();
     return;
   }

and then all the 82 frames get rendered, but the video renderer won't run the "ended" callback at all.
test-effective-equals-queued.log
313 KB View Download
Ah, in that case we'll have to iterate over the frames and figure out how many have a zero render_count and clamp the returned values to a minimum of that value when frame_dropping_disabled_ is true.
Owner: wdzierza...@opera.com
Status: Started (was: Untriaged)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1e43d3da2315557b19009165ea9c8a580d63eda

commit e1e43d3da2315557b19009165ea9c8a580d63eda
Author: wdzierzanowski <wdzierzanowski@opera.com>
Date: Thu Nov 24 16:47:05 2016

Adjust VideoRendererAlgorithm for |frame_dropping_disabled_|

This makes video frame hashing in tests immune to timing variations that
are inherent in the rendering algorithm.

BUG= 663709 
TEST=media_unittests pass, new unit test VideoRendererAlgorithmTest.EffectiveFramesQueuedWithoutFrameDropping

Review-Url: https://codereview.chromium.org/2502093002
Cr-Commit-Position: refs/heads/master@{#434350}

[modify] https://crrev.com/e1e43d3da2315557b19009165ea9c8a580d63eda/media/filters/video_renderer_algorithm.cc
[modify] https://crrev.com/e1e43d3da2315557b19009165ea9c8a580d63eda/media/filters/video_renderer_algorithm.h
[modify] https://crrev.com/e1e43d3da2315557b19009165ea9c8a580d63eda/media/filters/video_renderer_algorithm_unittest.cc

Status: Verified (was: Started)

Sign in to add a comment