• dalecurtis's avatar
    Partially revert change to not post AttemptRead() every Render(). · 95a43e33
    dalecurtis authored
    http://crrev.com/328649 changed VideoRendererImpl so that it only
    posted AttemptRead() when there was space in the queue.  Unfortunately
    this causes more dropped frames for demanding content (H.264 4K in
    particular) than always posting since the AttemptRead() call may be
    delayed for some time:
    
    AttemptRead() TimeFromPostUntilExecuted: 0.076 ms
    AttemptRead() TimeFromPostUntilExecuted: 4.613 ms
    AttemptRead() TimeFromPostUntilExecuted: 56.158 ms
    AttemptRead() TimeFromPostUntilExecuted: 56.197 ms
    AttemptRead() TimeFromPostUntilExecuted: 52.214 ms
    AttemptRead() TimeFromPostUntilExecuted: 35.555 ms
    AttemptRead() TimeFromPostUntilExecuted: 18.917 ms
    
    At the same time, the performance improvements I saw locally on my
    MacBook are not reflected by the performance bots.  I further see the
    dropped frame regression locally on Windows, so let's revert.
    
    I considered a couple other solutions to avoid always posting, but
    none were very glamorous:
    - Release lock before calling AttemptRead(), fixes cases where Render()
    was blocked on the lock in FrameReady().
    - Checking the end time / ideal render count for the current frame and
    posting on when we're almost expired...
    
    In both cases, neither resolved all problem cases and both add more
    complexity than just posting.  The task will abort trivially if there
    is no work to do, so just revert.
    
    BUG=439548
    TEST=telemetry dropped frame count is lower.
    
    Review URL: https://codereview.chromium.org/1135143002
    
    Cr-Commit-Position: refs/heads/master@{#329281}
    95a43e33
video_renderer_impl.cc 24.4 KB