Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 8 users
Status: Verified
Owner:
Email to this user bounced
Closed: Feb 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug-Regression

Blocking:
issue 106492
issue 108396

Restricted
  • Only users with EditIssue permission may comment.



Sign in to add a comment
Playback rates != 1.0 && (approximately) < 2 for HTML5 video mutes volume
Project Member Reported by vrk@chromium.org, Dec 20 2011 Back to list
What steps will reproduce the problem?
1. Play a <video>
2. Set playback rate to 0.5 or 1.5

What is the expected output? What do you see instead?
Should play at half or 1.5 times the normal playback rate. Instead, the volume is muted. Setting playback rate to 1.0, 2.0, 3.0, and 4.0 all work fine.

The bug was introduced in r113821 (http://codereview.chromium.org/8785008).
 
Comment 1 by vrk@chromium.org, Dec 21 2011
Blocking: 106492
Comment 2 by vrk@chromium.org, Dec 21 2011
Labels: Mstone-18
OK, I see what the problem is. The bug summary is slightly wrong, but I'll keep it the same since the problem is kind of hard to describe.

AudioRendererImpl calls FillBuffer with a different size now. Before FillBuffer was given the size of the shared memory; now it is given   
(bytes_per_sample * number_of_channels) bytes...

The real problem is, AudioRendererImpl is requesting bytes in terms of # of frames ("give me X frames of data"), whereas FillBuffer's internal implementation is thinking in terms of seconds ("output X seconds of data")... there are a few ways to fix this, so I will talk to people tomorrow for ideas!
Comment 3 by vrk@chromium.org, Dec 21 2011
Summary: Playback rates != 1.0 && (approximately) < 2 for HTML5 video mutes volume (was: NULL)
Reread my comment and realized it doesn't quite make sense. Here are some details:

AudioRendererImpl is requesting a number of samples based on |number_of_frames|, which is set in AudioDevice upon initialization. AudioRendererImpl initializes AudioDevice, and it sets it to the value of GetBufferSizeForSampleRate().

AudioRendererAlgorithmBase isn't thinking in terms of frames, and instead thinks in terms of seconds of data to output. In order to stretch/squish audio for playback rates != 1.0, FillBuffer() will usually require a buffer big enough to hold 0.08s of data. It appears the frames returned by GetBufferSizeForSampleRate() comprise less than 0.08s of data, so if playback rate is != 1.0 and less than approximately 2*, sound is muted.

*The amount of needed data for sped-up audio is inversely proportional to the playback rate, so at some point between 1.5 and 2 the playback rate is fast enough bring the needed data low enough to playback sound. Didn't bother finding out the original number.
Comment 4 by vrk@chromium.org, Jan 3 2012
Owner: ----
Status: Available
Not working on this right now; changing status to available.
Labels: -Mstone-18
 Issue 112417  has been merged into this issue.
Cc: crogers@google.com
Comment 8 by vrk@chromium.org, Feb 9 2012
Cc: alek...@chromium.org imasaki@chromium.org
 Issue 113131  has been merged into this issue.
Comment 9 by vrk@chromium.org, Feb 9 2012
 Issue 113042  has been merged into this issue.
Comment 10 by vrk@chromium.org, Feb 15 2012
Labels: -Pri-2 Pri-1 Mstone-18 ReleaseBlock-Stable
Status: Started
Owner: vrk@chromium.org
Comment 12 by vrk@chromium.org, Feb 18 2012
CL out: https://chromiumcodereview.appspot.com/9395057/

It's a significant but localized rewrite. It may be worth merging in as-is if this fixes a ton of other bugs, but otherwise we should think of safer CLs/hacks for M18.
Comment 13 by vrk@chromium.org, Feb 22 2012
I have another fix out for review that should be safer for M18:
https://chromiumcodereview.appspot.com/9416085/

CL 9395057 should still land, but hopefully in M19 instead of M18.
Labels: -Type-Bug Type-Regression
Project Member Comment 15 by bugdroid1@chromium.org, Feb 23 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=123292

------------------------------------------------------------------------
r123292 | vrk@google.com | Thu Feb 23 11:16:35 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=123292&r2=123291&pathrev=123292
 D http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/media/audio_common.cc?r1=123292&r2=123291&pathrev=123292
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/content_browser.gypi?r1=123292&r2=123291&pathrev=123292
 D http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/media/audio_common.h?r1=123292&r2=123291&pathrev=123292
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/audio_renderer_impl.cc?r1=123292&r2=123291&pathrev=123292
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/audio_util.cc?r1=123292&r2=123291&pathrev=123292
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/media/audio_renderer_host.cc?r1=123292&r2=123291&pathrev=123292
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/audio_util.h?r1=123292&r2=123291&pathrev=123292

Increase the buffer size in AudioRendererImpl to fix muted playback rate

This CL temporarily removes GetBufferSizeForSampleRate() and uses
SelectSamplesPerPacket() to determine the size of samples_per_packet in
AudioDevice. This restores the buffer size that AudioRendererImpl
used prior to r113821. SelectSamplesPerPacket() calculates a much larger
samples_per_packet value than GetBufferSizeForSampleRate(), so it fixes
the issue described in 108239.

This CL also does some mild refactoring to move audio_common.cc into
audio_util.cc.

BUG= 108239 
TEST=media_unittests,content_unittests, manual testing of different playback rates

Review URL: https://chromiumcodereview.appspot.com/9416085
------------------------------------------------------------------------
On YouTube/VideoTestMatrix, the sound still stops when I choose 1/16, 1/8x, 1/4x, 8x, 16x, 32x using Chrome 19.0.1051.0 canary on Windows7. Is this expected?

Comment 17 by vrk@chromium.org, Feb 24 2012
(talked offline but I'll update the bug for everyone following)

imasaki: Yes, right now we purposely mute the volume at playback rates less than 0.5 and greater than 4x. Though muted, the video itself should still play back at approximately the given rate. (This is how it worked in M17 and previous versions as well.) 
Cool. Fix is verified.  tweisberg@chromium.org is running sanity test with this fix.

Comment 19 by vrk@chromium.org, Feb 27 2012
Cc: tweisberg@chromium.org
Labels: Merge-Requested
Requesting merge for r123292, so long as tweisberg doesn't find anything crazy.

Ping tweisberg?
I'm not seeing anything new that would cause problems on Windows or Mac.  Sound plays for 1.5x and .5x speeds.  .25x speed doesn't play sound though.  All speeds work properly.
Linux test is done. This change is QA approved for merge.

Comment 22 by kareng@google.com, Feb 27 2012
Labels: -Merge-Requested Merge-Approved
Project Member Comment 23 by bugdroid1@chromium.org, Feb 27 2012
Labels: -merge-approved merge-merged-1025
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=123774

------------------------------------------------------------------------
r123774 | vrk@google.com | Mon Feb 27 11:09:21 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/media/audio/audio_util.h?r1=123774&r2=123773&pathrev=123774
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/content/browser/renderer_host/media/audio_input_renderer_host.cc?r1=123774&r2=123773&pathrev=123774
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/content/renderer/media/audio_renderer_impl.cc?r1=123774&r2=123773&pathrev=123774
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/content/content_browser.gypi?r1=123774&r2=123773&pathrev=123774
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/media/audio/audio_util.cc?r1=123774&r2=123773&pathrev=123774
 D http://src.chromium.org/viewvc/chrome/branches/1025/src/content/browser/renderer_host/media/audio_common.h?r1=123774&r2=123773&pathrev=123774
 M http://src.chromium.org/viewvc/chrome/branches/1025/src/content/browser/renderer_host/media/audio_renderer_host.cc?r1=123774&r2=123773&pathrev=123774
 D http://src.chromium.org/viewvc/chrome/branches/1025/src/content/browser/renderer_host/media/audio_common.cc?r1=123774&r2=123773&pathrev=123774

Merge 123292 - Increase the buffer size in AudioRendererImpl to fix muted playback rate

This CL temporarily removes GetBufferSizeForSampleRate() and uses
SelectSamplesPerPacket() to determine the size of samples_per_packet in
AudioDevice. This restores the buffer size that AudioRendererImpl
used prior to r113821. SelectSamplesPerPacket() calculates a much larger
samples_per_packet value than GetBufferSizeForSampleRate(), so it fixes
the issue described in 108239.

This CL also does some mild refactoring to move audio_common.cc into
audio_util.cc.

BUG= 108239 
TEST=media_unittests,content_unittests, manual testing of different playback rates

Review URL: https://chromiumcodereview.appspot.com/9416085

TBR=vrk@google.com
Review URL: https://chromiumcodereview.appspot.com/9479012
------------------------------------------------------------------------
The fix was merged and build into 18.0.1025.44. We will do sanity check using this build.

QA test is done on Mac/Win/Linux using 18.0.1025.44. No issue is found. ChromeOS build is not ready at this point. We will test it when the build is ready.
Blocking: 108396
Status: Verified
It is verified on 18.0.1025.45 on ChromeOS (Alex) as well. Putting this as verified.

Project Member Comment 28 by bugdroid1@chromium.org, Mar 6 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=125052

------------------------------------------------------------------------
r125052 | vrk@chromium.org | Mon Mar 05 16:48:37 PST 2012

Changed paths:
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/audio_hardware.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/content/renderer/media/audio_renderer_impl.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base_unittest.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_algorithm_base_unittest.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/seekable_buffer.h?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base.h?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/null_audio_renderer.h?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/audio_util.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_algorithm_base.h?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_base.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/null_audio_renderer.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/filters/audio_renderer_algorithm_base.cc?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/audio/audio_util.h?r1=125052&r2=125051&pathrev=125052
 M http://src.chromium.org/viewvc/chrome/trunk/src/media/base/seekable_buffer.cc?r1=125052&r2=125051&pathrev=125052

Fix muted audio when playback rate != 1.0 or 0.0

Rewrites the logic in AudioRendererAlgorithmBase to be able to output audio
at any point of a sped-up/slowed down window, instead of only outputting audio
in full multiples of windows.

BUG= 108239 
TEST=media_unittests, manual testing on video test matrix


Review URL: http://codereview.chromium.org/9395057
------------------------------------------------------------------------
Owner: tweisberg@chromium.org
Status: Fixed
Assign to tweisberg@ to verify the r125052.

Owner: vrk@chromium.org
Status: Verified
Working as intended on 19.0.1061.0 Canary.  Verified fixed.
Comment 31 Deleted
Project Member Comment 33 by bugdroid1@chromium.org, Oct 13 2012
Blocking: -chromium:106492 -chromium:108396 chromium:106492
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member Comment 34 by bugdroid1@chromium.org, Mar 9 2013
Labels: -Type-Regression -Area-WebKit -Feature-Media-Audio -Mstone-18 Cr-Content Type-Bug-Regression Cr-Internals-Media-Audio M-18
Project Member Comment 35 by bugdroid1@chromium.org, Mar 13 2013
Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member Comment 36 by bugdroid1@chromium.org, Apr 6 2013
Labels: -Cr-Content Cr-Blink
Sign in to add a comment