small-shot.ogg seems to play twice (again)

RESOLVED FIXED in mozilla12



6 years ago
6 years ago


(Reporter: cpearce, Assigned: cpearce)



Windows 7

Firefox Tracking Flags

(Not tracked)




(1 attachment)



6 years ago
When played in Firefox, content/media/test/small-shot.ogg seems to play twice per play(). Every other player I play small_shot.ogg in plays it once, so either we are wrong, or everybody else it.

I seem to remember having this problem before...

Comment 1

6 years ago
Regression range:

This is a regression from bug 636894;
Blocks: 636894
Keywords: regression


6 years ago
Assignee: nobody → cpearce

Comment 2

6 years ago
Created attachment 589350 [details] [diff] [review]
Patch v1

So we have two problems here:
1. We use minWriteFrames wrong. The code assumes that any individual call to nsAudioStream::Write() which writes less than minWriteFrames frames won't end up with all frames pushed to the hardware. On Windows this isn't the case. Audio is queued up in blocks of size minWriteFrames, once a block is filled, it gets written to hardware. So at any time if we've not written an even multiple of minWriteFrames frames, we'll have audio which has been queued but not written to hardware. This is what's happening while playing small-shot.ogg; we're writing minWriteFrames+N frames, and some of the audio data is overlapping and not completely filling the last block and so is not played, and then we're underrunning, causing the stuttery playback.
2. sa_stream_get_min_write isn't returning a value which is an even multiple of num_channels * bytes_per_sample, so the calculation in nsNativeAudioStream::GetMinWriteSize() is rounding down, and so our predictions of how much silence we need to write to ensure the last block of audio gets played are off by a few frames anyway.

1. Instead of pushing silence if the *last* write pushed less than minWriteFrames frames, push enough silence to ensure the number of frames we've pushed is a multiple of minWriteFrames, so all blocks are flushed.
2. Move the brackets around in sa_stream_create_pcm to ensure the block size is a multiple of num_channels*bytes_per_sample. 

Greenish on Try, modulo known randoms:
Attachment #589350 - Flags: review?(kinetik)
Comment on attachment 589350 [details] [diff] [review]
Patch v1

Review of attachment 589350 [details] [diff] [review]:

::: content/media/nsBuiltinDecoderStateMachine.cpp
@@ +782,5 @@
>        ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
> +      PRInt64 unplayedFrames = audioDuration % minWriteFrames;
> +      if (minWriteFrames > 1 && unplayedFrames > 0) {
> +        // Sound is written by libsydneyaudio to the hardware in blocks of
> +        // frames, of size minWriteFrames. So if the number of frames we've

Extra comma after frames.
Attachment #589350 - Flags: review?(kinetik) → review+

Comment 4

6 years ago
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
(In reply to Ed Morley [:edmorley] from comment #6)

This cset belongs in bug 713381, wrong bug number in commit.
You need to log in before you can comment on or make changes to this bug.