Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

[Android] Support third part media player on Crosswalk #353

Open
wants to merge 768 commits into
base: master
Choose a base branch
from

Conversation

fujunwei
Copy link

A requirement from an important customer who want to forward web resources with proxy
on Crosswalk, but android system MediaPlayer can't set a proxy with a standard API.
The ExoMediaPlayer is playing videos and music is a popular activity on Android devices,
and it can be configured with proxy.
https://developer.android.com/guide/topics/media/exoplayer.html

BUG=XWALK-6770

@crosswalk-trybot
Copy link

crosswalk-trybot commented May 17, 2016

Testing patch series with fujunwei/chromium-crosswalk@50e6ea0 as its head.

Bot Status
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/286)
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/284)

@fujunwei fujunwei force-pushed the new_exo_media_player branch from 50e6ea0 to 1756745 Compare May 17, 2016 07:13
@crosswalk-trybot
Copy link

crosswalk-trybot commented May 17, 2016

Testing patch series with fujunwei/chromium-crosswalk@1756745 as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/285)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/287)

@fujunwei fujunwei force-pushed the new_exo_media_player branch from 1756745 to 076c594 Compare May 17, 2016 07:20
@crosswalk-trybot
Copy link

crosswalk-trybot commented May 17, 2016

Testing patch series with fujunwei/chromium-crosswalk@076c594 as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/286)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/288)

huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request May 17, 2016
… size of the fullscreen chrome window by 1 px on activation loss.

Merging to M50

We reduce the size of fullscreen windows by 1px to ensure that maximized windows on the same thread don't draw over the
taskbar. This change caused a painting problem as the compositor was not aware of the changed size. This causes the compositor
to not paint correctly when the fullscreen window is activated as the window size did not change.

Setting the compositor size correctly to the window bounds size fixes this problem. The other bug I found was when the
fullscreen window is activated, we inform the delegate about the changed client size which in turn makes it across to the
webcontents. We don't want the webcontents to get notified about these size changes. Fixed by setting the flag background_fullscreen_hack_
to false after the SetBoundsInternal call.

BUG=595666
TBR=sky

Review URL: https://codereview.chromium.org/1819633002

Cr-Commit-Position: refs/heads/master@{#382429}
(cherry picked from commit 9f060d4)

Conflicts:
	ui/views/win/hwnd_message_handler.cc

Review URL: https://codereview.chromium.org/1826703002 .

Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#353}
Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
@fujunwei fujunwei force-pushed the new_exo_media_player branch from 076c594 to b3b820f Compare May 18, 2016 00:49
@crosswalk-trybot
Copy link

crosswalk-trybot commented May 18, 2016

Testing patch series with fujunwei/chromium-crosswalk@b3b820f as its head.

Bot Status
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/289)
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/287)

@fujunwei fujunwei force-pushed the new_exo_media_player branch from b3b820f to c1a63a3 Compare May 18, 2016 01:03
@crosswalk-trybot
Copy link

crosswalk-trybot commented May 18, 2016

Testing patch series with fujunwei/chromium-crosswalk@c1a63a3 as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/290)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/288)

@fujunwei
Copy link
Author

@rakuco @lincsoon @axinging @wuhengzhi PTAL.

@baleboy
Copy link

baleboy commented May 23, 2016

Crosswalk 20 branch date is May 27th, please prioritize the review if you want this in.

protected MediaPlayer getLocalPlayer() {
if (mPlayer == null) {
mPlayer = new MediaPlayer();
protected ExMediaPlayer getExMediaPlayer() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to change name, getLocalPlayer will also be ok.
Is there an easy way to switch from MediaPlayer to ExoMediaPlayer, , co-exist?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it better to create ExMediaPlayerBridge.java to reduce the rebase workload?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How to use it in MediaPlayerBridge?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree to keep the original name to reduce rebase workload.

@wuhengzhi
Copy link
Contributor

How to set the proxy for player? I can not find the interface.

@fujunwei
Copy link
Author

The API will to be exposed in another PR in crosswalk repo.

fujunwei pushed a commit to fujunwei/chromium-crosswalk that referenced this pull request May 27, 2016
Ensure the suggestion list container is initialized before attempting
to add things to it.

BUG=565113

Review URL: https://codereview.chromium.org/1516273004

Cr-Commit-Position: refs/heads/master@{#364886}

Review URL: https://codereview.chromium.org/1528693002 .

Cr-Commit-Position: refs/branch-heads/2564@{crosswalk-project#353}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
@fujunwei fujunwei force-pushed the new_exo_media_player branch from c1a63a3 to b22e688 Compare June 3, 2016 02:37
@crosswalk-trybot
Copy link

crosswalk-trybot commented Jun 3, 2016

Testing patch series with fujunwei/chromium-crosswalk@b22e688 as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/293)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/291)

@fujunwei
Copy link
Author

fujunwei commented Jun 3, 2016

The PR of exposing api to developer is crosswalk-project/crosswalk#3736

@fujunwei
Copy link
Author

fujunwei commented Jun 3, 2016

@rakuco @lincsoon @axinging @wuhengzhi PTAL

@@ -0,0 +1,123 @@
// Copyright 2013 The Chromium Authors. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2016, add Intel's copyright?

@fujunwei fujunwei force-pushed the new_exo_media_player branch from b22e688 to 679ff18 Compare June 3, 2016 06:31
@crosswalk-trybot
Copy link

crosswalk-trybot commented Jun 3, 2016

Testing patch series with fujunwei/chromium-crosswalk@679ff18 as its head.

Bot Status
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/294)
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/292)

@fujunwei
Copy link
Author

fujunwei commented Jun 3, 2016

Thanks, done.

@fujunwei fujunwei force-pushed the new_exo_media_player branch from 679ff18 to 05d75eb Compare June 3, 2016 08:43
@crosswalk-trybot
Copy link

crosswalk-trybot commented Jun 3, 2016

Testing patch series with fujunwei/chromium-crosswalk@05d75eb as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/295)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/293)

getLocalPlayer(), context, uri)) {
if (sResourceLoadFilter != null
&& sResourceLoadFilter.shouldOverrideResourceLoading(
getLocalPlayer(), context, uri)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you changed nothing but still left a diff record here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for passing style checks.

@fujunwei
Copy link
Author

fujunwei commented Jun 8, 2016

@rakuco @halton @lincsoon @axinging @wuhengzhi Do you have any updates?

Lin Sun and others added 15 commits August 16, 2016 15:27
Unlike WebView, XWalkView uses a ContentView object as its child view,
which is actually receiving all kinds of events such as touching and
clicking. To let the XWalkView object receive these events too, it's
necessary to override relevant methods of ContentView and redirect
them to the XWalkView object. So we add XWalkContentView and have it
extend ContentView. To make this possible, the contructor of ContentView
must be public.

XWalkContentView is a temporary solution. We are targeting to optimize
the hierarchy of XWalkView to make it more like WebView. This task can
be tracked via XWALK-6118.

BUG=XWALK-6014
Add Intel RSSDK based video capture device VideoCaptureDeviceRSWin.

It allows:
1. Crosswalk apps to share the RealSense camera with other native apps
   simultaneously.
2. Crosswalk apps to use getUserMedia API to preview RealSense camera
   in real-time while accessing RSSDK middlewares in Crosswalk extensions
   simultaneously.

This feature is behind "use_rssdk" build flag and "use-rs-video-capture"
runtime flag.

TEST=
On a device with RealSense camera and RSSDK, execute
1. DF_RawStreams.exe (RSSDK sample)
2. xwalk.exe --use-rs-video-capture
   https://webrtc.github.io/samples/src/content/devices/input-output/
Crosswalk and DF_RawStreams.exe should capture video frames from
RealSense camera simultaneously.

BUG=XWALK-6028
BUG=XWALK-5503

Some OSs (e.g. Windows and Linux) don't mark windows as hiden on screen
lock or when other opaque windows fully cover them.
Note that e.g. OSX and Android do this while Windows and Linux don't.
OnSoftVisibilityChanged enables that OS window state is kept as is (on
Linux and Windows) and that e.g. power saving PageVisibility API is still
properly triggered.

The first patch here targets Windows and screen lock.

branch pagevis
[windows] App does not support orientation lock

On Windows 8 and later, fullscreen is not mandatory to get application
prefered locking.
Patch here implements Screen orientation lock API for Windows.

BUG=XWALK-5002
If set the WindowBackground of Activity to RED, and webpage page's backgroudn to RED,
when application starts or resumes after click back key, it will showup a white screen
shortly. With this SetBackgroundColor, we can change the first screen to a fix color,
which can help to avoid white screen.

BUG=XWALK-4809, XWALK-4995

Conflicts:
	content/browser/android/content_view_core_impl.cc

[M50 Rebase: SetBackgroundColor() was removed in https://crrev.com/1651933003.
add back for content/browser/android/content_view_core_impl.h]
This setting, introduced in 070dd8c ("[Android] Rework multidex and
enable multidex for unit_tests_apk. (RELAND 2)"), caused our test APKs
to fail to build:

  Uncaught translation error: java.lang.IllegalArgumentException: already added: Lorg/chromium/base/multidex/ChromiumMultiDex;

For now, revert the `generate_multidex_config` part of the change while
we investigate how to make things work without this commit.

BUG=XWALK-6547
BUG=XWALK-6625
Enable transparent TextureView when user changes the default SurfaceView to TextureView.

BUG=XWALK-6519
OverScrolled Event could be catch by WebView through Android View
System. But XWalkView is not a true view, so XWalkView need this patch
to port OverScrolled Event to it.

BUG=XWALK-4871
BUG=XWALK-4894
Those headers will be imported into chromium-crosswalk, so we need to
stop ignoring the directory.

BUG=XWALK-6661
These headers correspond to
https://cvs.khronos.org/svn/repos/registry/trunk/public/cl/api/1.2@30030.

The headers used to be pulled by Crosswalk's DEPS.xwalk, but with M49
the WebCL code that uses the OpenCL headers is always built, so we need
them in chromium-crosswalk for content_shell builds to work.

BUG=XWALK-6661
…ribute

The newly added 'AudioDestinationNode.devicePosition' attribute
helps to bind audio context time and performance time values.

Please see for more details:
WebAudio/web-audio-api#754
WebAudio/web-audio-api#12

For M51 rebase adapted to,
https://codereview.chromium.org/1773813007
Before this change calling of AudioOutputStream::AudioSourceCallback
method without stream position ended up in an empty implementation.

BUG=XWALK-6703
Those two files were erroneously added in commit bbc2a01 ("[Temp] Do not
enable generate_multidex_config by default on Android").

BUG=XWALK-6547
BUG=XWALK-6625
BUG=XWALK-6961
**IMPORTANT**
We actually intend to fix this properly. We are cherry-picking this
commit from the crosswalk-18 branch first in order to have a stable
baseline with no WebVR support propagating from Crosswalk 21 to 20 to
19. After that, we can start working on fixing this in 21 and 20 and, if
there is enough time, 19 too.

*From the original commit message:*

Move `CardboardVRDevice.java` to a different directory so that it is not
built by the `content_java` target even if `enable_webvr==0`.

This is a prerequisite for getting the code to build with
`enable_webvr=0`.

BUG=XWALK-6746

(cherry picked from commit e593418)
FontRendererStyle was excluded.

Fixing https://codereview.chromium.org/1944993003/ issue that missed
FontRendererStyle.cpp in gyp file
@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 22, 2016

Testing patch series with fujunwei/chromium-crosswalk@ff1ef6c as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/328)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/326)

@darktears
Copy link
Contributor

LGTM.

@rakuco ?

@rakuco
Copy link

rakuco commented Aug 25, 2016

Not yet, IMO.

  • The questions that must always be asked on every single chromium-crosswalk pull request and which have not been addressed here: have you tried to upstream this in any form? If not, why not? What's the maintenance plan here, are we expected to carry this patch forever?
  • "I want to integrate this patch because my unnamed, important customer wants it" is not a valid reason to have a patch merged into a public, open-source project. You need to come up with a better explanation of why we want this.

Code-wise: isn't it possible to add a delegate that returns a subclass of MediaPlayer so you don't need to define ExternalMediaPlayer at all? Something like making getLocalPlayer() return either a new MediaPlayer or a subclass if one is registered in Crosswalk. This way the change to chromium-crosswalk is much smaller (and easier to get accepted upstream) and you make XWalkMediaPlayer inherit from MediaPlayer and let your users change its behavior as they see fit.

@fujunwei fujunwei force-pushed the new_exo_media_player branch from ff1ef6c to 61b8ec9 Compare August 31, 2016 01:07
@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 31, 2016

Testing patch series with fujunwei/chromium-crosswalk@61b8ec9 as its head.

Bot Status
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/336)
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/334)

@fujunwei
Copy link
Author

fujunwei commented Aug 31, 2016

The questions that must always be asked on every single chromium-crosswalk pull request and which have not been addressed here: have you tried to upstream this in any form? If not, why not? What's the maintenance plan here, are we expected to carry this patch forever?

I will communicate with upstream to figure out whether it's acceptable feature.

"I want to integrate this patch because my unnamed, important customer wants it" is not a valid reason to have a patch merged into a public, open-source project. You need to come up with a better explanation of why we want this.

This feature can't be got directly from Android System WebView, there are requirements to build custom media player using ExoPlayer. for example, customizing full screen video, user interface and adding media codec such as ijkplayer.

isn't it possible to add a delegate that returns a subclass of MediaPlayer so you don't need to define ExternalMediaPlayer at all?

The approach has a demerit that an invalid MediaPlayer is created by default, but the third party MediaPlayer don't need it usually such as Google ExoPlayer. The refined codes is also committed, PTAL.

@fujunwei fujunwei force-pushed the new_exo_media_player branch from 61b8ec9 to 8fcaeac Compare August 31, 2016 01:48
@crosswalk-trybot
Copy link

crosswalk-trybot commented Aug 31, 2016

Testing patch series with fujunwei/chromium-crosswalk@8fcaeac as its head.

Bot Status
Content Shell Android-x86 [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/337)
Content Shell Linux [SUCCESS 💚](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/335)

A requirement from an important customer who want to forward web resources with proxy
on Crosswalk, but android system MediaPlayer can't set a proxy with a standard API.
The ExoMediaPlayer is playing videos and music is a popular activity on Android devices,
and it can be configured with proxy.
https://developer.android.com/guide/topics/media/exoplayer.html

BUG=XWALK-6770
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request Oct 9, 2016
Previously removing the SafePointScope had a risk of producing a dead lock if the following scenario happens:

1) The main thread starts a GC and tries to stop all other threads.
2) The worker thread is waiting on a signal without entering the SafePointScope. The main thread cannot stop the worker thread forever and thus cannot make progress. Dead lock.

However, (to avoid this kind of dead lock) code has been added to timeout the GC if the GCing thread fails at stopping other threads within 100 ms. So not entering SafePointScope will just cause the timeout -- it won't cause a dead lock.

This fixes a hard-to-reproduce timing bug that's been a big concern of one of our partners.

BUG=592124,608603
TEST=manual

Review-Url: https://codereview.chromium.org/2009093002
Cr-Commit-Position: refs/heads/master@{#399265}
(cherry picked from commit f466a07)

Review URL: https://codereview.chromium.org/2065823003 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#353}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
imreotto pushed a commit to tenta-browser/chromium-crosswalk that referenced this pull request Nov 2, 2017
Disable sending kTabModelNewTabWillOpenNotification notification
when the TabModel is restoring a session as this breaks the BVC
state that does not expect the notification to be sent when a tab
is added due to session restoration.

This is a reland of http://crrev.com/c/664557 that fixes EG tests
by correctly initialising TabModelNotificationObserver (should not
be disabled except during the restoration of the session).

This CL also improves on the original CL by using a scoped closure
runner to re-enable TabModelNotificationObserver to protect against
early returns in -restoreSessionWindow:persistState:.

Bug: 763964
Change-Id: Ie950ac3f35b13566abcc1ca6eae774512ed7a16a
Reviewed-on: https://chromium-review.googlesource.com/665238
Reviewed-by: Rohit Rao (ping after 24h) <[email protected]>
Commit-Queue: Sylvain Defresne <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#501944}(cherry picked from commit e8ae6ad)
Reviewed-on: https://chromium-review.googlesource.com/674983
Reviewed-by: Sylvain Defresne <[email protected]>
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#353}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
@SonicACCEL
Copy link

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.