The default bug view has changed. See this FAQ.
Bug 623444 (cubeb)

Replace libsydneyaudio with a callback based audio library

RESOLVED FIXED in mozilla15

Status

()

Core
Audio/Video
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: kinetik, Assigned: kinetik)

Tracking

(Depends on: 1 bug, Blocks: 2 bugs)

Trunk
mozilla15
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 13 obsolete attachments)

15.66 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
32.81 KB, patch
khuey
: review+
Details | Diff | Splinter Review
(Assignee)

Description

6 years ago
libsydneyaudio's API is not a good fit for our uses and forces us to use an unsatisfactory design for audio playback in the media playback engine.  The plan is to replace this as soon as possible after Firefox 4 is released.  I'll write some more about the plan shortly.
(Assignee)

Comment 1

6 years ago
The plan is to build a small library that maps closely to modern sound APIs (PulseAudio on Linux, CoreAudio (AudioQueue) on OS X, and either DirectSound or XAudio2 on Windows).  Rather than exposing a push-to-play model like sydneyaudio (which effectively requires a separate thread per playing media element to write audio), a callback will be called when more audio is required.

It may be necessary to provide an ALSA backend in addition to the PulseAudio one, depending on the minimum Linux distro requirements post Firefox 4.

Requirements:
- remove necessity for 1-2 threads per active audio stream that we currently have (bug 592833)
- underrun behavior/handling should be the same on each platform
- low latency volume changes (bug 487504)
- sharing top-level audio resources (bug 617852)
- correct playback of sub-one-buffer length audio chunks (bug 615452)
(Assignee)

Updated

6 years ago
Blocks: 542635
(Assignee)

Updated

6 years ago
Blocks: 521615
(Assignee)

Updated

6 years ago
Blocks: 617852
(Assignee)

Updated

6 years ago
Blocks: 487504
(Assignee)

Comment 2

6 years ago
Reviewing some important aspects of existing behaviour:

Playback start:
- Linux: when start threshold reached
- OS X: after first write
- Win32: when block filled

With a callback based model, this problem mostly disappears.  The library can decide what an appropriate initial buffer size is and explicitly request that the application fill it.

Underrun:
- Linux: audio clock stops ticking
- OS X: audio clock continues at normal rate (callback writes silence)
- Win32: audio clock stops ticking

It's preferable for the clock to stop ticking.  The only reason it doesn't on OS X is that I'm not aware of a way to stop it--the implementation is already using a callback model, and when a callback requests more data than is available, it's not obvious what you can do other than write silence.

API calls while underrun:
- Linux: return error, usually return useless (initial state) values after recovery
- OS X: work as there's effectively no underrun state
- Win32: return frozen state at point of underrun

Ideally the underrun recovery should be dealt with in a single place, so other API calls should try to return sensible values when playback is stopped due to underrun.

Underrun recovery:
- Linux: explicit, expensive, partial buffers lost
- OS X: N/A
- Win32: implicit (write more), cheap, no buffers lost

There's not much control available of this, it's an attribute of the OS's audio API.

Clock granularity:
- Linux: unknown (but probably discoverable), often not more than one period length
- OS X: dependent on callback buffer request size
- Win32: unknown, seems fairly high (likely sample accurate)

Clock read cost:
- Linux: medium to high, cross-DSO call, possibly involving IPC
- OS X: low, reads a local counter protected by a lock
- Win32: medium, cross-DSO call

In the current implementation, the caller may call the clock read function in a tight loop (with very short sleeps in between).  In the older Firefox media playback engine, it was assumed that the clock was fine enough granularity that individual video frames could be timed based on the audio clock updates.  Either the clock granularity must be explicit, high, and fast to read, or the API must be designed in such a way that the application can't easily make inappropriate assumptions about the clock's behaviour.

Updated

6 years ago
Blocks: 639587
(Assignee)

Updated

6 years ago
Blocks: 640405

Comment 3

6 years ago
Please don't declare PulseAudio "THE Linux audio API". OSS4 is way better. So if you design your library, please make the audio backend optional, i.e. able to support ALSA, OSS4 or whatever else. Don't forget also that other Unixes don't use PulseAudio (while OSS4 is available for example for FreeBSD and Solaris).
Adding my voice here, pulseaudio and oss4 are not available for OpenBSD (not even speaking of alsa), so i'll need to submit a backend for our sndio API. See https://bugzilla.mozilla.org/show_bug.cgi?id=648726 for the sydneyaudio backend.
(Assignee)

Updated

6 years ago
Depends on: 662417
(Assignee)

Updated

6 years ago
Blocks: 620721
(Assignee)

Updated

6 years ago
Blocks: 620598
(Assignee)

Updated

6 years ago
Blocks: 610931
(Assignee)

Updated

6 years ago
Blocks: 610930
(Assignee)

Updated

6 years ago
Blocks: 591790
(Assignee)

Comment 5

6 years ago
OS X test builds available here: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mgregan@mozilla.com-631bc0100998/try-macosx64/

Linux test builds will be available once bug 662417 is resolved.

Win32 test builds will be available once I've fixed a couple more bugs (later this week, I hope).
OS: Linux → All
Hardware: x86_64 → All
(Assignee)

Updated

6 years ago
Blocks: 652485
And what about the others ? Is there a 'dummy fallback backend' or a way to just disable audio, until corresponding backends are written ? Is the 'new api' documented/published somewhere, so we can start working on it ?
(Assignee)

Comment 7

6 years ago
There's a build time switch to disable compilation of the new code and a run time pref (for testing purposes) to switch between the new code and the current code.  Those will both be removed at some point in the future (along with libsydneyaudio), so getting non-tier 1 platforms supported by the new code will need to happen before that removal.

Based on what we've got for libsydneyaudio, the additional backends needed are:
AIX
ALSA - I'll work on this once the initial work is done.
Android - As above, unless I can find someone else.
OpenBSD (sndio)
OS/2
OSS
Solaris (sunaudio)

API is here: http://flim.org/~kinetik/cubeb.h

If you have any questions, let me know via email or IRC.
(Assignee)

Updated

6 years ago
Blocks: 665344
Blocks: 666092
Blocks: 666271
(Assignee)

Comment 8

6 years ago
Created attachment 544676 [details] [diff] [review]
remove support for fake non-blocking writes v0
(Assignee)

Updated

6 years ago
Attachment #544676 - Attachment description: remove support for fake non-blocking writes → remove support for fake non-blocking writes v0
(Assignee)

Comment 9

6 years ago
Created attachment 544678 [details] [diff] [review]
rename samples to audio frames v0

Rename internal uses of "samples" to "audio frames" to make it clearer what unit is being operated on.
(Assignee)

Comment 10

6 years ago
Created attachment 544679 [details] [diff] [review]
import libcubeb v0

Import libcubeb (PulseAudio and OS X backends only, for now) and integrate with the Mozilla build system.
(Assignee)

Comment 11

6 years ago
Created attachment 544681 [details] [diff] [review]
implement libcubeb based nsAudioStream v0

A libcubeb based nsAudioStream is needed to support the current Audio Data API and remote audio code, and will be temporarily used by the builtin decoders for playback.  This allows testing libcubeb backends with minimal changes to the decoder logic, and provides a pref (media.use_cubeb) to switch between the old and new code.  The remote audio and decoder audio code will be replaced with implementations that use libcubeb directly in short order.
(Assignee)

Comment 12

6 years ago
The Windows implementation is (still) missing because there are problems with the DirectSound implementation I built that I haven't been able to solve.  The main problem is that it's impossible (as far as I can tell) to queue multiple secondary buffers to play back seamless, so I used a single DSBPLAY_LOOPING circular buffer which is refilled as soon as a quarter of it has emptied.  The problem with this approach is that if the refill thread misses the refill deadline, buffer playback wraps and it becomes impossible to accurately calculate the true playback position.  This is partially worked around by using a system timer to guess if the playback within circular buffer has looped, but the end result is not robust enough.  It also has the undesirable attribute that the user hears whatever random audio is left in the buffer upon buffer underrun.

As a short term measure, I'm building a WaveOut API version which I'll post very early next week.  After that, I'll look at building an XAudio2 implementation (which allows multiple buffers to be queued for seamless playback) and using that where possible.

Comment 13

6 years ago
I think this also blocks bug 587465 (can't seem to update blocks/depends on).
(Assignee)

Updated

6 years ago
Blocks: 671535
(Assignee)

Updated

6 years ago
Attachment #544676 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #544678 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #544681 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #544679 - Attachment is obsolete: true
(Assignee)

Comment 14

6 years ago
Created attachment 553645 [details] [diff] [review]
patch v0

Implement nsAudioStream interface using libcubeb.  The media.use_cubeb hidden pref (which defaults off for now) can be used to enable or disable use of the cubeb-based stream.
Attachment #553645 - Flags: review?(chris)
Comment on attachment 553645 [details] [diff] [review]
patch v0

Review of attachment 553645 [details] [diff] [review]:
-----------------------------------------------------------------

Can you make sure http://flim.org/~kinetik/cubeb.h is up to date? That would make reviewing this easier.

::: content/media/nsAudioStream.cpp
@@ +816,5 @@
> +
> +  long DataCallback(void* aBuffer, long aFrames);
> +  int StateCallback(cubeb_state aState);
> +
> +  Monitor mMonitor;

Add a comment saying what's protected by monitor.

@@ +818,5 @@
> +  int StateCallback(cubeb_state aState);
> +
> +  Monitor mMonitor;
> +
> +  PRInt64 mLostFrames;

Definintely need a comment on mLostFrames. It's a sum of the silence cubeb injected when we underran?

@@ +833,5 @@
> +  SampleFormat mFormat;
> +  PRUint32 mBytesPerFrame;
> +
> +  PRPackedBool mPaused;
> +  PRPackedBool mRunning;

Call mRunning something different to make it obvious it's not the same as !mPaused.

@@ +900,5 @@
> +  }
> +
> +  {
> +    cubeb_stream* stream;
> +    if (cubeb_stream_init(gCubebContext, &stream, "nsBufferedAudioStream", params, params.rate / 5,

Seems the magic 5 should be a #define or constant.

@@ +911,5 @@
> +    return NS_ERROR_FAILURE;
> +  }
> +
> +  mBufferLimit = aRate * mBytesPerFrame;
> +  mBufferLimit += mBytesPerFrame - (mBufferLimit % mBytesPerFrame);

Since |mBufferLimit = aRate * mBytesPerFrame|, isn't (mBufferLimit % mBytesPerFrame) always 0?

@@ +1031,5 @@
> +  }
> +
> +  uint64_t position = 0;
> +  cubeb_stream_get_position(mCubebStream, &position);
> +  PRInt64 adjusted = NS_MAX(PRInt64(0), PRInt64(position - mLostFrames));

This will be called on the state machine thread, so the read of mLostFrames needs to be synchronized by the monitor.

@@ +1081,5 @@
> +    memset(static_cast<PRUint8*>(aBuffer) + bytesToCopy, 0, framesToCopy * mBytesPerFrame);
> +    mLostFrames += framesToCopy;
> +    framesToCopy -= framesToCopy;
> +  }
> +  return aFrames - framesToCopy;

In your example in http://flim.org/~kinetik/cubeb.h you return CUBEB_OK from your data callback, why not here? Maybe you could add some appropriately named temporary variables or comments to make it more obvious what's going on here.

@@ +1085,5 @@
> +  return aFrames - framesToCopy;
> +}
> +
> +int
> +nsBufferedAudioStream::StateCallback(cubeb_state aState)

Can't this callback run after the audio thread writes data but before the audio thread calls Drain()? If that happens, won't Drain() hang forever?
(Assignee)

Comment 16

6 years ago
Created attachment 554789 [details] [diff] [review]
bufferedaudiostream patch v1

Updated patch addressing review feedback.

> Since |mBufferLimit = aRate * mBytesPerFrame|, isn't (mBufferLimit % mBytesPerFrame) always 0?
Yes, it's a leftover from when mBufferLimit was calculated as |aRate * mBytesPerFrame / 2|, which required rounding.  I'll remove this line and leave the assertion.

> In your example in http://flim.org/~kinetik/cubeb.h you return CUBEB_OK from your data callback, why not here?
Up-to-date version is here: https://github.com/kinetiknz/cubeb/blob/master/include/cubeb/cubeb.h

The API was changed so that the callee returns the number of frames copied to the provided buffer.  If the result is less than the number of frames requested, the stream enters drain state.

> Can't this callback run after the audio thread writes data but before the audio thread calls Drain()? If that happens, won't Drain() hang forever?

The stream only enters drain state once the data callback returns less than the requested number of frames.  This can't happen until Drain() sets mDraining = true.
Attachment #553645 - Attachment is obsolete: true
Attachment #554789 - Flags: review?(chris)
Attachment #553645 - Flags: review?(chris)
Comment on attachment 554789 [details] [diff] [review]
bufferedaudiostream patch v1

Review of attachment 554789 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/nsAudioStream.cpp
@@ +1121,5 @@
> +  case FORMAT_S16_LE:
> +    SampleCopy<PRInt16>(aBuffer, mBuffer.Elements(), samplesToCopy, mVolume);
> +    break;
> +  case FORMAT_FLOAT32:
> +    SampleCopy<float>(aBuffer, mBuffer.Elements(), samplesToCopy, mVolume);

Can you also multiply mVolume by GetVolumeScale() in these SampleCopy() calls so that the media.volume_scale pref still works with your new stream?
Attachment #554789 - Flags: review?(chris) → review+
Blocks: 683263
(Assignee)

Updated

6 years ago
Depends on: 669556
(Assignee)

Updated

6 years ago
Attachment #554789 - Attachment description: patch v1 → bufferedaudiostream patch v1
(Assignee)

Updated

6 years ago
Depends on: 689432
(Assignee)

Updated

6 years ago
Attachment #554789 - Attachment is obsolete: true
(Assignee)

Comment 18

6 years ago
Created attachment 562652 [details] [diff] [review]
bufferedaudiostream patch v2

Two minor changes:
- avoid an unwanted lock recursion attempt in StateCallback
- draining a stream that hasn't started should complete immediately
Attachment #562652 - Flags: review?(chris)
(Assignee)

Updated

6 years ago
Attachment #562652 - Attachment description: patch v2 → bufferedaudiostream patch v2
(Assignee)

Comment 19

6 years ago
Comment on attachment 562652 [details] [diff] [review]
bufferedaudiostream patch v2

Obsoleting, because this is hitting the following assertion on tryserver:
###!!! ASSERTION: Stream pause in unexpected state.: 'mState == STARTED', file e:/builds/moz2_slave/try-w32-dbg/build/content/media/nsAudioStream.cpp, line 1033
Attachment #562652 - Attachment is obsolete: true
Attachment #562652 - Flags: review?(chris)
(Assignee)

Comment 20

6 years ago
Created attachment 562907 [details] [diff] [review]
bufferedaudiostream v3

Same changes as listed for the last patch, plus this makes ::Pause and ::Resume behave the same as the existing code for unstarted streams.
Attachment #562907 - Flags: review?(chris)
(Assignee)

Comment 21

6 years ago
Created attachment 562980 [details] [diff] [review]
import libcubeb v1

Import libcubeb.  This includes working OS X (AudioQueue) and Win32 (winmm) implementations.  The build machinery is in place for a Linux (ALSA) version, but I've excluded the file from this patch until I complete some refactoring.
Attachment #562980 - Flags: review?(chris.double)
Comment on attachment 562907 [details] [diff] [review]
bufferedaudiostream v3

Review of attachment 562907 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. r=cpearce assuming that nsBufferedAudioStream::mBufferLimit can never be ridiculously large (see below).

::: content/media/nsAudioStream.cpp
@@ +844,5 @@
> +  // Owning reference to a cubeb_stream.  cubeb_stream_destroy is called by
> +  // nsAutoRef's destructor.
> +  nsAutoRef<cubeb_stream> mCubebStream;
> +
> +  PRInt32 mRate;

Is it worth making mRate and mChannels PRUInt32? These are unsigned in cubeb_stream_params. r+ regardless of whether you do or not.

@@ +853,5 @@
> +  enum StreamState {
> +    INITIALIZED, // Initialized, playback has not begun.
> +    STARTED,     // Started by a call to Write() (iff INITIALIZED) or Resume().
> +    STOPPED,     // Stopped by a call to Pause().
> +    DRAINING,    // Drain requested.  DataCallback will signal end of stream

Is this comment right? Did you mean to say "StateCallback will be called to signal end of stream..." instead of "DataCallback"... ?

@@ +918,5 @@
> +    params.format = CUBEB_SAMPLE_FLOAT32LE;
> +    mBytesPerFrame = sizeof(float) * aNumChannels;
> +    break;
> +  default:
> +      return NS_ERROR_FAILURE;

Looks like indentation is off on the return here.

@@ +936,5 @@
> +
> +  // Limit mBuffer to one second of audio.  This value is arbitrary, and was
> +  // selected based on the observed behaviour of the existing nsAudioStream
> +  // implementations.
> +  mBufferLimit = aRate * mBytesPerFrame;

Am I right to assume nsBufferedAudioStream::mBufferLimit can never be ridiculously large because cubeb_stream_init() will fail if passed in ridiculous values for rate and channel? Do we need to cap this value?

@@ +1131,5 @@
> +
> +  // Remove copied data from the temporary audio buffer.
> +  mBuffer.RemoveElementsAt(0, available);
> +  NS_ABORT_IF_FALSE(mBuffer.Length() % mBytesPerFrame == 0, "Must copy complete frames");
> +

Remove extra newline here?
Attachment #562907 - Flags: review?(chris) → review+

Updated

6 years ago
Attachment #562980 - Flags: review?(chris.double) → review+
(Assignee)

Comment 23

6 years ago
Created attachment 563304 [details] [diff] [review]
bufferedaudiostream v4

Address review comments, carrying forward review.
Attachment #562907 - Attachment is obsolete: true
Attachment #563304 - Flags: review+
(Assignee)

Updated

6 years ago
Blocks: 694484
(Assignee)

Updated

6 years ago
Blocks: 698328
(Assignee)

Updated

5 years ago
Blocks: 582513
(Assignee)

Comment 24

5 years ago
Created attachment 578167 [details] [diff] [review]
bufferedaudiostream v5

Rebased and PR_BOOL converted.  Carrying forward review.
Attachment #563304 - Attachment is obsolete: true
Attachment #578167 - Flags: review+
(Assignee)

Comment 25

5 years ago
Created attachment 578169 [details] [diff] [review]
import libcubeb v2

This version includes the ALSA backend.  The only other changes are very minor:
- Added sanity checks to cubeb_stream_init.
- Removed bogus assert from wmme backend.

content/media mochitests pass locally on Linux with bare ALSA and ALSA with the PulseAudio plugin.  They also pass locally on Win32 and OS X, but there are some test timeouts on OS X on try that I'm investigating now.
Attachment #562980 - Attachment is obsolete: true
Attachment #578169 - Flags: review?(chris.double)
(Assignee)

Comment 26

5 years ago
Comment on attachment 578169 [details] [diff] [review]
import libcubeb v2

The libcubeb code corresponds to git commit ID fc6e838a0078a10ecc306f5c671c07ec2bfe7607.  README_MOZILLA has the wrong ID because I imported the code before I committed the final changes.
(Assignee)

Updated

5 years ago
No longer blocks: 591790, 610930, 610931, 620721, 665344, 666092, 666271, 671535
(Assignee)

Comment 27

5 years ago
Comment on attachment 578169 [details] [diff] [review]
import libcubeb v2

Some last minute locking changes to the ALSA backend are causing problems on the try machines.  Cancelling review until I solve them.
Attachment #578169 - Flags: review?(chris.double)
(Assignee)

Comment 28

5 years ago
Created attachment 588168 [details] [diff] [review]
bufferedaudiostream v6

Other than rebasing, the only change from the already review v5 is that I moved the media.use_cubeb pref stuff so that it shares the thread safe observing with media.volume_scale, since I was previously fetching media.use_cubeb (unsafely) off the main thread.
Attachment #578167 - Attachment is obsolete: true
Attachment #588168 - Flags: review?(chris)
(Assignee)

Comment 29

5 years ago
Created attachment 588170 [details] [diff] [review]
import libcubeb v3 win32 only

Import just the Win32 backend (already reviewed in v2) for now.  This just needs build system integration review before landing.
Attachment #578169 - Attachment is obsolete: true
Attachment #588170 - Flags: review?(khuey)
Comment on attachment 588168 [details] [diff] [review]
bufferedaudiostream v6

Review of attachment 588168 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/nsAudioStream.cpp
@@ +343,5 @@
> +      NS_ConvertUTF16toUTF8 utf8(value);
> +      gVolumeScale = NS_MAX<double>(0, PR_strtod(utf8.get(), nsnull));
> +    }
> +  } else if (strcmp(aPref, PREF_USE_CUBEB) == 0) {
> +    bool value = Preferences::GetBool(aPref, true);

Maybe this should be GetBool(aPref, false), otherwise we'll be using cubeb by default when MOZ_CUBEB is defined? We can then enable it platform-by-platform for desktop in firefox.js, and eventually set the pref in all.js once cubeb is stable on mobile (and B2G? I'm not sure if B2G loads all.js...).

@@ +885,5 @@
> +    INITIALIZED, // Initialized, playback has not begun.
> +    STARTED,     // Started by a call to Write() (iff INITIALIZED) or Resume().
> +    STOPPED,     // Stopped by a call to Pause().
> +    DRAINING,    // Drain requested.  DataCallback will indicate end of stream
> +                 // once the remaining contents of mBuffer have requested by

s/have requested/have been requested/  ?

@@ +894,5 @@
> +
> +  StreamState mState;
> +
> +  // Arbitrary default stream latency.  The higher this value, the longer stream
> +  // volume changes will take to become audible.

The lower the value....  ?

@@ +931,5 @@
> +
> +nsresult
> +nsBufferedAudioStream::Init(PRInt32 aNumChannels, PRInt32 aRate, SampleFormat aFormat)
> +{
> +  if (!gCubebContext || aNumChannels < 0 || aRate < 0) {

Maybe <= 0 instead? Can we succeed with aNumChannels or aRate equal to 0?
Attachment #588168 - Flags: review?(chris) → review+
(Assignee)

Comment 31

5 years ago
Comment on attachment 588170 [details] [diff] [review]
import libcubeb v3 win32 only

Clearing build review for now; this needs to be updated to inclue libcubeb in the gkmedias library (bug 709721).
Attachment #588170 - Flags: review?(khuey)
(Assignee)

Comment 32

5 years ago
(In reply to Chris Pearce, Mozilla Corporation (:cpearce) from comment #30)
> Maybe this should be GetBool(aPref, false), otherwise we'll be using cubeb
> by default when MOZ_CUBEB is defined? We can then enable it
> platform-by-platform for desktop in firefox.js, and eventually set the pref
> in all.js once cubeb is stable on mobile (and B2G? I'm not sure if B2G loads
> all.js...).

That's intentional.  libcubeb will be enabled by default for each backend as the code is added to the tree (Win32 is being added first), and may then be entirely disabled via the pref when branching for Aurora, depending on how stable the code is.

> @@ +885,5 @@
> > +    INITIALIZED, // Initialized, playback has not begun.
> > +    STARTED,     // Started by a call to Write() (iff INITIALIZED) or Resume().
> > +    STOPPED,     // Stopped by a call to Pause().
> > +    DRAINING,    // Drain requested.  DataCallback will indicate end of stream
> > +                 // once the remaining contents of mBuffer have requested by
> 
> s/have requested/have been requested/  ?

Thanks, will fix.

> The lower the value....  ?

The higher the requested latency, the more data is buffered at the OS level after the point at which we apply the per-stream volume adjustment, so the longer it'll take for that to play out and any change in the per-stream volume to be submitted to the OS for playback.

> Maybe <= 0 instead? Can we succeed with aNumChannels or aRate equal to 0?

libcubeb/cubeb_stream_init is responsible for validating the stream parameters.  This check is only validating that the params can be converted to unsigned integers.
(Assignee)

Comment 33

5 years ago
Created attachment 588363 [details] [diff] [review]
import libcubeb v4 win32 only

Previously reviewed code, just needs build system review.  This version is integrated with the gkmedias library on Win32.  Try run was as green as usual.
Attachment #588170 - Attachment is obsolete: true
Attachment #588363 - Flags: review?(khuey)
Can we please use mozilla/StdInt.h when it's available? Shouldn't be too hard to add something like http://mxr.mozilla.org/mozilla-central/source/mfbt/StdInt.h#65
Comment on attachment 588363 [details] [diff] [review]
import libcubeb v4 win32 only

Review of attachment 588363 [details] [diff] [review]:
-----------------------------------------------------------------

Build bits look fine.  Sorry this took so long.

::: media/libcubeb/Makefile.in
@@ +46,5 @@
> +
> +DIRS		= \
> +		include \
> +		src \
> +		$(NULL)

No tabs please.

::: media/libcubeb/src/Makefile.in
@@ +48,5 @@
> +ifeq (WINNT,$(OS_TARGET))
> +VISIBILITY_FLAGS =
> +endif
> +
> +ifneq (,$(filter WINNT WINCE,$(OS_ARCH)))

WINCE is long dead, no need for that here.

@@ +51,5 @@
> +
> +ifneq (,$(filter WINNT WINCE,$(OS_ARCH)))
> +CSRCS           = \
> +                cubeb_winmm.c \
> +                $(NULL)

No tabs please.
Attachment #588363 - Flags: review?(khuey) → review+
(Assignee)

Comment 36

5 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/71678cfd06dd
http://hg.mozilla.org/integration/mozilla-inbound/rev/7d1f570d9136

(In reply to Ms2ger from comment #34)
> Can we please use mozilla/StdInt.h when it's available? Shouldn't be too
> hard to add something like
> http://mxr.mozilla.org/mozilla-central/source/mfbt/StdInt.h#65

I changed cubeb-stdint.h to simply include mozilla/StdInt.h.
Whiteboard: [leave open after merge]
https://hg.mozilla.org/mozilla-central/rev/7d1f570d9136
(Assignee)

Comment 38

5 years ago
https://hg.mozilla.org/mozilla-central/rev/71678cfd06dd was pushed with bug number 677138, but actually belongs to bug 623444 (this bug).
(Assignee)

Updated

5 years ago
Whiteboard: [leave open after merge]
(Assignee)

Updated

5 years ago
Depends on: 723793
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla13
(Assignee)

Updated

5 years ago
Depends on: 723781
(Assignee)

Comment 39

5 years ago
This is causing a fatal assertion during tests (see bug 723781).

Disabled: http://hg.mozilla.org/integration/mozilla-inbound/rev/27f0028890e7
(Assignee)

Updated

5 years ago
Whiteboard: [leave open after merge]
(Assignee)

Updated

5 years ago
Whiteboard: [leave open after merge]

Updated

5 years ago
Depends on: 731807
Blocks: 668449
(Assignee)

Updated

5 years ago
Blocks: 732046
(Assignee)

Updated

5 years ago
Depends on: 742154
(Assignee)

Updated

5 years ago
Depends on: 742160
(Assignee)

Updated

5 years ago
Depends on: 747793
(Assignee)

Updated

5 years ago
Depends on: 751030
(Assignee)

Updated

5 years ago
Depends on: 756944
Depends on: 757034
(Assignee)

Updated

5 years ago
Alias: cubeb
(Assignee)

Updated

5 years ago
Depends on: 758198
(Assignee)

Updated

5 years ago
Depends on: 759821
(Assignee)

Updated

5 years ago
Depends on: 759677
(Assignee)

Updated

5 years ago
Blocks: 589595
(Assignee)

Comment 40

5 years ago
Marking this as fixed since the bulk of the work is complete.  Everything else is in follow up bugs.
No longer blocks: 589595
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
No longer depends on: 759677
Resolution: --- → FIXED
Version: Trunk → Other Branch
(Assignee)

Updated

5 years ago
Blocks: 589595
Depends on: 759677
Target Milestone: mozilla13 → mozilla15
Version: Other Branch → Trunk

Comment 41

5 years ago
What is the sample count accuracy for each OS? I see the Web Audio API for Google Chrome allows js devs to explicitly use 256 sample buffers (And I personally use the 2048 samples per buffer @ 44.1khz). mozCurrentSampleOffset relies on buffer position accuracy, so we might need charts up in the wiki. Lastly, could this buffer size (for positon accuracy) be exposed to moz audio? It seems mac really is accurate, while linux and windows suffer some loss of accuracy in position determination.
(Assignee)

Updated

5 years ago
Depends on: 783052
(Assignee)

Updated

5 years ago
No longer blocks: 639587
(Assignee)

Updated

5 years ago
Depends on: 788005
(Assignee)

Updated

5 years ago
Depends on: 791112
You need to log in before you can comment on or make changes to this bug.