Closed
Bug 805254
Opened 13 years ago
Closed 13 years ago
Simplify audio format handling
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: roc, Assigned: roc)
Details
Attachments
(13 files)
4.08 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
5.39 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
2.63 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
7.89 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
6.26 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
7.05 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
18.04 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
21.35 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
2.86 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
2.65 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
4.03 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
7.46 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
7.63 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
Now that we hardcode the output audio sample format at compile time (to MOZ_AUDIO_DATA_FORMAT), we can simplify some media code.
Assignee | ||
Comment 1•13 years ago
|
||
We can eliminate support for FORMAT_U8 completely. AFAIK only WAVE files contain these and we already convert them to MOZ_AUDIO_DATA_FORMAT in the WAVE reader.
On desktop we could work with float32 at all times in AudioData and MediaStreams, so we wouldn't even need to support buffers with varying sample formats. Unfortunately on mobile we want to use S16 for media playback but we need to use float32 for Web Audio, so we'll still need to deal with sample formats and conversion in MediaStreams. We could have AudioData use S16 only and MediaStreams use float32 only, but I think it's good to allow MediaStreams to be used for media playback without converting samples, so let's continue to let them be S16 or floats. Also WebRTC wants S16.
Assignee | ||
Comment 2•13 years ago
|
||
Attachment #675041 -
Flags: review?(kinetik)
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #675042 -
Flags: review?(kinetik)
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #675043 -
Flags: review?(kinetik)
Assignee | ||
Comment 5•13 years ago
|
||
Attachment #675044 -
Flags: review?(kinetik)
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #675045 -
Flags: review?(kinetik)
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #675046 -
Flags: review?(kinetik)
Assignee | ||
Comment 8•13 years ago
|
||
Attachment #675047 -
Flags: review?(kinetik)
Assignee | ||
Comment 9•13 years ago
|
||
I didn't do a particularly good job of equalizing patch sizes ... the other patches are all pretty small and focused, but this one snowballed a bit. Sorry.
Attachment #675050 -
Flags: review?(kinetik)
Assignee | ||
Comment 10•13 years ago
|
||
Attachment #675051 -
Flags: review?(kinetik)
Assignee | ||
Comment 11•13 years ago
|
||
Attachment #675052 -
Flags: review?(kinetik)
Assignee | ||
Comment 12•13 years ago
|
||
Attachment #675053 -
Flags: review?(kinetik)
Assignee | ||
Comment 13•13 years ago
|
||
Attachment #675054 -
Flags: review?(kinetik)
Assignee | ||
Comment 14•13 years ago
|
||
Attachment #675055 -
Flags: review?(kinetik)
Assignee | ||
Comment 15•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #675047 -
Attachment description: Bug 806254. Part 7: Move SampleFormat to mozilla::AudioSampleFormat → Part 7: Move SampleFormat to mozilla::AudioSampleFormat
Updated•13 years ago
|
Attachment #675041 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675042 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675043 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675044 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675045 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675046 -
Flags: review?(kinetik) → review+
Comment 16•13 years ago
|
||
Comment on attachment 675047 [details] [diff] [review]
Part 7: Move SampleFormat to mozilla::AudioSampleFormat
The SharedBuffer.h change doesn't seem to belong in this patch.
Attachment #675047 -
Flags: review?(kinetik) → review+
Comment 17•13 years ago
|
||
Comment on attachment 675050 [details] [diff] [review]
Part 8: Consolidate audio sample processing code
GetSampleSize could be replaced with the trait mechanism, but it's not a big deal either way.
Attachment #675050 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 18•13 years ago
|
||
There's a build failure due to the 16-bit InterleaveAndConvertBuffer not being used on desktop (and warnings-as-errors being set). I'm not sure of the best way to suppress that...
Assignee | ||
Comment 19•13 years ago
|
||
(In reply to Matthew Gregan [:kinetik] from comment #17)
> GetSampleSize could be replaced with the trait mechanism, but it's not a big
> deal either way.
It goes away in part 12 anyway.
(In reply to Matthew Gregan [:kinetik] from comment #16)
> The SharedBuffer.h change doesn't seem to belong in this patch.
It actually is needed because SharedBuffer.h uses nsCOMPtr/nsRefPtr and some #include change means that they're no longer defined in time for SharedBuffer to use them.
Updated•13 years ago
|
Attachment #675051 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675052 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675053 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 20•13 years ago
|
||
Updated•13 years ago
|
Attachment #675054 -
Flags: review?(kinetik) → review+
Updated•13 years ago
|
Attachment #675055 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 21•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9102f7a79c0
https://hg.mozilla.org/integration/mozilla-inbound/rev/c30be4a4cd06
https://hg.mozilla.org/integration/mozilla-inbound/rev/afb0fa607ef2
https://hg.mozilla.org/integration/mozilla-inbound/rev/4edf6b315c50
https://hg.mozilla.org/integration/mozilla-inbound/rev/870743d33a56
https://hg.mozilla.org/integration/mozilla-inbound/rev/da7b1a168146
https://hg.mozilla.org/integration/mozilla-inbound/rev/569b5cf14285
https://hg.mozilla.org/integration/mozilla-inbound/rev/882cfaba69c2
https://hg.mozilla.org/integration/mozilla-inbound/rev/caff86361591
https://hg.mozilla.org/integration/mozilla-inbound/rev/68faa49eec3d
https://hg.mozilla.org/integration/mozilla-inbound/rev/c3fcdbf50104
https://hg.mozilla.org/integration/mozilla-inbound/rev/1eb3bd54b7bd
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b496b04c183
Comment 22•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b9102f7a79c0
https://hg.mozilla.org/mozilla-central/rev/c30be4a4cd06
https://hg.mozilla.org/mozilla-central/rev/afb0fa607ef2
https://hg.mozilla.org/mozilla-central/rev/4edf6b315c50
https://hg.mozilla.org/mozilla-central/rev/870743d33a56
https://hg.mozilla.org/mozilla-central/rev/da7b1a168146
https://hg.mozilla.org/mozilla-central/rev/569b5cf14285
https://hg.mozilla.org/mozilla-central/rev/882cfaba69c2
https://hg.mozilla.org/mozilla-central/rev/caff86361591
https://hg.mozilla.org/mozilla-central/rev/68faa49eec3d
https://hg.mozilla.org/mozilla-central/rev/c3fcdbf50104
https://hg.mozilla.org/mozilla-central/rev/1eb3bd54b7bd
https://hg.mozilla.org/mozilla-central/rev/2b496b04c183
Status: NEW → RESOLVED
Closed: 13 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in
before you can comment on or make changes to this bug.
Description
•