Closed
Bug 932112
Opened 11 years ago
Closed 11 years ago
Update WebRTC code to webrtc.org stable branch 3.43
Categories
(Core :: WebRTC, defect)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: jesup, Assigned: glandium)
References
Details
Attachments
(6 files, 2 obsolete files)
(deleted),
patch
|
froydnj
:
review+
glandium
:
checkin+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jesup
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mfinkle
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
blassey
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
3.43 is being used in Chrome 31, to be released soon.
We'll keep the new AEC code on top of it
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #823780 -
Flags: review?(nfroyd)
Assignee | ||
Updated•11 years ago
|
Assignee: rjesup → mh+mozilla
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Attachment #823780 -
Flags: review?(nfroyd)
Assignee | ||
Comment 2•11 years ago
|
||
Attachment #823787 -
Flags: review?(nfroyd)
Assignee | ||
Updated•11 years ago
|
Attachment #823780 -
Attachment is obsolete: true
Updated•11 years ago
|
Attachment #823787 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Whiteboard: [leave open]
Comment 4•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #823787 -
Flags: checkin+
Reporter | ||
Comment 5•11 years ago
|
||
Reporter | ||
Comment 6•11 years ago
|
||
Comment 7•11 years ago
|
||
Comment on attachment 827769 [details] [diff] [review]
Add a non-ARM MemoryBarrier function. r=
Why not just use __sync_synchronize() all the time? Less ugly and the compiler will basically do the same thing (or perhaps it'll be hidden in the runtime somewhere) as your casting hack.
Reporter | ||
Comment 8•11 years ago
|
||
Reporter | ||
Comment 9•11 years ago
|
||
Reporter | ||
Comment 10•11 years ago
|
||
Reporter | ||
Comment 11•11 years ago
|
||
Reporter | ||
Comment 12•11 years ago
|
||
Reporter | ||
Comment 13•11 years ago
|
||
Comment on attachment 827769 [details] [diff] [review]
Add a non-ARM MemoryBarrier function. r=
Note: this patch is used for opensles drivers, which means android and gonk only.
Attachment #827769 -
Flags: review?(mh+mozilla)
Reporter | ||
Updated•11 years ago
|
Attachment #827770 -
Flags: review?(blassey.bugs)
Reporter | ||
Updated•11 years ago
|
Attachment #827781 -
Flags: review+
Reporter | ||
Updated•11 years ago
|
Attachment #827782 -
Flags: review?(blassey.bugs)
Reporter | ||
Updated•11 years ago
|
Attachment #827783 -
Flags: review?(blassey.bugs)
Reporter | ||
Comment 14•11 years ago
|
||
Tested with our internal tests, apprtc, gupshup, and some others. (Can't test with talky.io because of bug 934378)
Let's get it in before bitrot ensues!
Assignee | ||
Comment 15•11 years ago
|
||
Comment on attachment 827769 [details] [diff] [review]
Add a non-ARM MemoryBarrier function. r=
Review of attachment 827769 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/trunk/webrtc/modules/audio_device/android/single_rw_fifo.cc
@@ +25,5 @@
> // barrier.
> typedef void (*KernelMemoryBarrierFunc)();
> ((KernelMemoryBarrierFunc)0xffff0fa0)();
> +#else
> + __sync_synchronize();
I agree with Nathan, you can just use __sync_synchronize() everywhere.
Attachment #827769 -
Flags: review?(mh+mozilla) → feedback+
Reporter | ||
Comment 16•11 years ago
|
||
Reporter | ||
Updated•11 years ago
|
Attachment #827769 -
Attachment is obsolete: true
Reporter | ||
Comment 17•11 years ago
|
||
Comment on attachment 827845 [details] [diff] [review]
Add a non-ARM MemoryBarrier function. r=
Per IRC
Attachment #827845 -
Flags: review?(mh+mozilla)
Assignee | ||
Updated•11 years ago
|
Attachment #827845 -
Flags: review?(mh+mozilla) → review+
Comment 18•11 years ago
|
||
Comment on attachment 827770 [details] [diff] [review]
Don't free the global, shared Context ref. r=
Review of attachment 827770 [details] [diff] [review]:
-----------------------------------------------------------------
I think I already r+'d this
Attachment #827770 -
Flags: review?(blassey.bugs) → review+
Comment 19•11 years ago
|
||
Comment on attachment 827782 [details] [diff] [review]
JB reflect for low-latency params r=
Review of attachment 827782 [details] [diff] [review]:
-----------------------------------------------------------------
I wrote this patch, so I probably shouldn't review it
Attachment #827782 -
Flags: review?(blassey.bugs) → review?(mark.finkle)
Comment 20•11 years ago
|
||
Comment on attachment 827782 [details] [diff] [review]
JB reflect for low-latency params r=
Transition to reflection looks like it covers the bases
Attachment #827782 -
Flags: review?(mark.finkle) → review+
Comment 21•11 years ago
|
||
Comment on attachment 827783 [details] [diff] [review]
Use the non-main-thread FindClass implementation r=
Review of attachment 827783 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_manager_jni.cc
@@ +85,4 @@
> // Create a global reference such that the class object is not recycled by
> // the garbage collector.
> + g_audio_manager_class_ = jsjni_GetGlobalClassRef(
> + "org/webrtc/voiceengine/AudioManagerAndroid");
What thread is this being run on? It will dispatch a runnable synchronously to the main thread, which means it will block waiting for the main thread. Is that ok?
Reporter | ||
Comment 22•11 years ago
|
||
Comment on attachment 827783 [details] [diff] [review]
Use the non-main-thread FindClass implementation r=
Review of attachment 827783 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/trunk/webrtc/modules/audio_device/android/audio_manager_jni.cc
@@ +85,4 @@
> // Create a global reference such that the class object is not recycled by
> // the garbage collector.
> + g_audio_manager_class_ = jsjni_GetGlobalClassRef(
> + "org/webrtc/voiceengine/AudioManagerAndroid");
> What thread is this being run on? It will dispatch a runnable synchronously to the main thread, which means it will block waiting for the main thread. Is that ok?
This will end up running on the MediaThread (the backend MediaManager's thread for interfacing with the media code). Blocking it on MainThread (once? or once per GetUserMedia call?) probably isn't a huge problem.
Updated•11 years ago
|
Attachment #827783 -
Flags: review?(blassey.bugs) → review+
Reporter | ||
Comment 23•11 years ago
|
||
Ready for checkin
https://tbpl.mozilla.org/?tree=Try&rev=236239b6f7be
Reporter | ||
Comment 24•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7aca035355ae
https://hg.mozilla.org/integration/mozilla-inbound/rev/bd8f1571937f
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b8a157a6ed5
https://hg.mozilla.org/integration/mozilla-inbound/rev/32d82a576615
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0e8a46aad5f
https://hg.mozilla.org/integration/mozilla-inbound/rev/cc7e3ef4bb80
https://hg.mozilla.org/integration/mozilla-inbound/rev/4481461159f2
Reporter | ||
Updated•11 years ago
|
Whiteboard: [leave open]
Comment 25•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7aca035355ae
https://hg.mozilla.org/mozilla-central/rev/bd8f1571937f
https://hg.mozilla.org/mozilla-central/rev/5b8a157a6ed5
https://hg.mozilla.org/mozilla-central/rev/32d82a576615
https://hg.mozilla.org/mozilla-central/rev/d0e8a46aad5f
https://hg.mozilla.org/mozilla-central/rev/cc7e3ef4bb80
https://hg.mozilla.org/mozilla-central/rev/4481461159f2
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•8 years ago
|
Blocks: webrtc_updates
You need to log in
before you can comment on or make changes to this bug.
Description
•