RESOLVED FIXED 102545
Enable media stream on Android for Chromium
https://bugs.webkit.org/show_bug.cgi?id=102545
Summary Enable media stream on Android for Chromium
wjia
Reported 2012-11-16 11:57:46 PST
This is part of effort to support WebRTC in Chrome on Android.
Attachments
Patch (1.18 KB, patch)
2012-11-18 21:34 PST, wjia
no flags
Patch (1.70 KB, patch)
2012-11-19 10:25 PST, wjia
no flags
Patch (1.64 KB, patch)
2012-12-14 09:12 PST, wjia
no flags
Patch (1.75 KB, patch)
2012-12-17 10:23 PST, wjia
no flags
wjia
Comment 1 2012-11-18 21:34:17 PST
Peter Beverloo
Comment 2 2012-11-19 05:17:21 PST
This will expose the feature, but will it actually work already? If I remember correctly, the run-time flag for media stream support defaults to being enabled, so unless it does work it'll break feature detection. I'd also like to know what kind of impact this has on the binary size. Together with WebRTC (and any needed non-ffmpeg components), it could add a significant amount of data. The leading WebRTC issue on Chromium for Android can be found at http://crbug.com/156384. I'll reply to the one you filed (http://crbug.com/161417) so we can have this discussion on the Chromium tracker instead.
wjia
Comment 3 2012-11-19 07:17:48 PST
It takes many steps to make WebRTC work on Android. This is the first one which enables all necessary components to build. MEDIA_STREAM in WebKit is just one of the needed components. After that, we need to make sure all components work and add the missing components such as video capture device. Regarding code size, that's a known issue. I'd have PM to discuss size issue.
Adam Barth
Comment 4 2012-11-19 09:51:29 PST
Comment on attachment 174886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174886&action=review > Source/WebKit/chromium/features.gypi:166 > + 'ENABLE_MEDIA_STREAM=1', You should move this outside the conditional block since it isn't conditional anymore.
Adam Barth
Comment 5 2012-11-19 09:52:29 PST
I'd like Peter to give and LGTM before we land this patch to make sure that he's happy with the change.
Adam Barth
Comment 6 2012-11-19 09:52:57 PST
s/and/an/
wjia
Comment 7 2012-11-19 10:25:08 PST
Peter Beverloo
Comment 8 2012-11-21 09:17:28 PST
Comment on attachment 175001 [details] Patch I'm removing r? for now. We're holding off with enabling Media Stream until after the Chrome 25 branch point. Thank you for uploading this patch, Wei Jia! We'll be able to land this later on.
wjia
Comment 9 2012-12-14 09:12:28 PST
wjia
Comment 10 2012-12-14 09:39:08 PST
Peter, The 3rd patch (id=179491) tries to set ENABLE_MEDIA_STREAM based on enable_webrtc. This puts all webrtc related stuff under one single flag. On Android, enable_webrtc is set to 0. So ENABLE_MEDIA_STREAM is set to 0, too. This will not change chromium on Android.
Peter Beverloo
Comment 11 2012-12-17 10:15:20 PST
Comment on attachment 179491 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=179491&action=review This seems good to me, with one minor nit. Thank you! > Source/WebKit/chromium/ChangeLog:8 > + Put all webrtc related stuff under one flag "enable_webrtc". nit: Could you clarify in the ChangeLog that the "enable_webrtc" flag is enabled for Linux, Mac and Windows builds, but disabled for Android?
wjia
Comment 12 2012-12-17 10:23:07 PST
wjia
Comment 13 2012-12-17 10:24:40 PST
Source/WebKit/chromium/ChangeLog has been updated according to the comment.
Peter Beverloo
Comment 14 2012-12-18 10:41:52 PST
+Eric for review.
Eric Seidel (no email)
Comment 15 2012-12-18 11:32:09 PST
I lean on tc for all my gyp reviews. :) But this LGTM too.
WebKit Review Bot
Comment 16 2012-12-18 11:59:19 PST
Comment on attachment 179764 [details] Patch Clearing flags on attachment: 179764 Committed r138049: <http://trac.webkit.org/changeset/138049>
WebKit Review Bot
Comment 17 2012-12-18 11:59:23 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.