novoda/no-player

Simplified Player wrapper for MediaPlayer and ExoPlayer

This project is no longer under maintenance - 13/7/2020

Upgrades to latests version of ExoPlayer requires a significative amount of changes. The project is no longer maintained from our end.

---

A simplified Android Player wrapper for MediaPlayer and ExoPlayer.

Description

Some of the benefits are:

• Unified playback interface and event listeners for ExoPlayer and MediaPlayer
• MediaPlayer buffering
• ExoPlayer local, streaming and provisioning WideVine Modular DRM
• Maintains video Aspect Ratio by default
• Player selection based on ContentType and DRM

Experimental Features, use with caution:

• Support for TextureView

Adding to your project

To start using this library, add these lines to the build.gradle of your project:

repositories {
    jcenter()
}

dependencies {
    implementation 'com.novoda:no-player:<latest-version>'
}

From no-player 4.5.0 this is also needed in the android section of your build.gradle

compileOptions {
        targetCompatibility JavaVersion.VERSION_1_8
}

Simple usage

1. Create a Player:

   Player player = new PlayerBuilder()
     .withPriority(PlayerType.EXO_PLAYER)
     .withWidevineModularStreamingDrm(drmHandler)
     .build(this);

2. Create the PlayerView:

   <com.novoda.noplayer.NoPlayerView
     android:id="@+id/player_view"
     android:layout_width="wrap_content"
     android:layout_height="wrap_content"
     android:layout_gravity="center" />

3. Attach to a PlayerView:

   PlayerView playerView = findViewById(R.id.player_view);
   player.attach(playerView);

4. Play some content:

   player.getListeners().addPreparedListener(playerState -> player.play());
   
   Uri uri = Uri.parse(mpdUrl);
   player.loadVideo(uri, ContentType.DASH);

Snapshots

Snapshot builds from develop are automatically deployed to a repository that is not synced with JCenter. To consume a snapshot build add an additional maven repo as follows:

repositories {
    maven {
        url 'https://dl.bintray.com/novoda-oss/snapshots/'
    }
}

You can find the latest snapshot version following this link.

Contributing

We always welcome people to contribute new features or bug fixes, here is how.

If you have a problem, check the Issues Page first to see if we are already working on it.

Looking for community help? Browse the already asked Stack Overflow Questions or use the tag support-no-player when posting a new question.

no-player's Issues

Initialise HashMaps with given size

Problem

As per @zegnus comment here, our HashMaps in the InfoForwarders are not initialised to any given size but use the default.

Potential Solution

Initialise our maps with the correct size.

Impact

Reduce memory footprint.

Can this repo be made open source?

Align portrait video to center-horizontal

I want to align the video to center horizontal within the playerView. Current portrait mode videos are shown Left aligned in my app. How can I do that ?

Thanks for the help.

Support for cache

Problem

At the moment the current implementation is not allowing any sort of caching mechanism. This could be really useful for some clients with strict offline behavior on applications without a download feature or on devices with dodgy connectivity

Potential Solution

I don't know about AndroidMediaPlayer
About ExoPlayer it should be fairly straightforward to supply a CacheDataSourceFactory upon creation of NoPlayer via NoPlayerExoPlayerCreator. The configuration for the cache can be done via Options, for example Options.withCache

Impact

As said in the problem, could make the playback more reliable on situations of doggy network connectivity and be a quick win for those clients with needs for offline behavior

PlayerFactory use a Builder

Problem

Lot's of "discussions" around the PlayerFactory when adding the DOWNGRADE_SECURE_DECODER because we passed a boolean all the way down instead of having two different method signatures.

Potential Solution

Use the Builder pattern, it's ideal when you want to customise something.

Impact

Extensible and we can provide easily visible defaults.

NoPlayerView use generic views

Problem

NoPlayerView relies on views that are exposed through the ExoPlayer package. If we were to ever drop support for ExoPlayer we would need to rework the NoPlayerView to remove the unsupported views.

Potential Solution

Implement our own versions of the ExoPlayer views that will have a longer life support.

Impact

We are not using code specific to one implementation in the other implementation which could be problematic when providing support to clients.

Null checks on AndroidMediaPlayerFacadeListeners

Problem

There are null checks on listeners present in the AndroidMediaPlayerFacade, most of these appear to be redundant because they are set by us, not the client, in the AndroidMediaPlayerImpl.

Potential Solution

Check which listeners have null checks, see where they are set and whether or not we can remove the check. If we can, remove the null check.

Impact

We will remove redundant code making the class smaller and easier to read.

NoPlayerLog NPE

Problem

The NoPlayerLog always assumes a trace file name is present but this may not be the case 👻

Fatal Exception: java.lang.NullPointerException: Attempt to invoke virtual method 'int java.lang.String.length()' on a null object reference
       at com.novoda.noplayer.internal.utils.NoPlayerLog.getDetailedLog(Unknown Source:28)
       at com.novoda.noplayer.internal.utils.NoPlayerLog.logMessage(Unknown Source:2)
       at com.novoda.noplayer.internal.utils.NoPlayerLog.e(Unknown Source:8)
       at com.novoda.noplayer.internal.listeners.StateChangedListeners.remove(Unknown Source:8)
       at com.novoda.noplayer.internal.exoplayer.ExoPlayerTwoImpl.pause(Unknown Source:11)

Potential Solution

Should we crash, or provide a default empty message?

Impact

A runtime exception will be triggered swallowing the original player error whilst trying to log the player error.

Player DRM interfaces are empty

The DrmHandler and the ClassicDrm are empty due to the drm being acquired at an earlier stage, before the player is created, this is not case for modular drm which will need to be passed to the player.

The code needs to be reworked to make sense since the classic drm is no longer required to be triggered by the Player

(Moved over from the project of origin)

Remove notils dependency

Problem

We're including the no-tils library for a single Exception

no-player/core/src/main/java/com/novoda/noplayer/PlayerType.java

Line 26 in 83b2aca

throw new DeveloperError("Can't create PlayerType from : " + rawPlayerType);

Potential Solution

To replace with our own custom exception.

Impact

• Decreases maintenance cost of having to keep notils up to date
• If clients were catching this error they will crash

Player Capabilities

Problem

The underlying Player can vary in the features that they offer. There is currently no easy way to select a Player based on a client's requirements, except for DRM.

Potential Solution

Introduce Capabilities, where a Player defines its own capabilities and we can match to these based on the client's requirements.

Impact

More tailored approach to selecting a Player.

Add api to change volume/mute

Problem

We do not currently expose setting the player volume.

Impact

• Need investigate behaviour of mediaplayer vs exoplayer

support audio playback

Problem

Currently the library only exposes playback of Video content although mediaplayer and exoplayer fully support audio playback (the library can be tricked by passing mp3 as a H264 content type).

noplayer also enforces a surface to be attached which is not necessary for audio content, eg playing in the background.

Potential Solution

• To not enforce a surface to be attached for audio content.
• Reuse the exoplayer extractor source for a new Audio content type
• Make a generic loadContent(uri, options) or provide separate loadVideo & loadAudio

Impact

Will allow noplayer to be used for non video purposes.

Test NoPlayerAdsLoader

Problem

NoPlayerAdsLoader has a lot of state and logic. We rely quite heavily on this and it is essentially untested.

Potential Solution

Move all logic to another collaborator that is testable.

Impact

Fewer bugs and documentation on how the AdsLoader works. We'll be able to more easily deal with production issues if we can reproduce them with tests.

Annotate interfaces for better Kotlin interoperability

Problem

Kotlin compiler doesn't have enough information to infer which methods are Nullable and which are NotNull

Potential Solution

Add NotNull annotations to the public API (or convert the whole library to Kotlin )

Impact

It will just make the library more kotlin-friendly. It's a low impact - low effort kind of thing

Remove deprecated TextOutput method

Problem

exoPlayer.setTextOutput(textRendererOutput.output()); has been deprecated by ExoPlayer.

Potential Solution

Use the new recommended method #addTextOutput(TextRenderer.Output) and test in client application.

Impact

Deprecated method, no guarantee that it will continue to be supported in the future.

Demo Player controls

Problem

We have no way of interacting with the Player.

Potential Solution

Implement a basic controller that includes:

• Elapsed time.
• Total duration.
• Play / pause button.
• Scrub bar.

Impact

There is no way to test edge cases involving scrubbing, play / pausing without deploying to a client application.

Simplify NoPlayer Listeners & Forwarders

Problem

There are many duplications around all the types of listener. A listener interface for each, a forwarder for each, a "collection of listeners" for each (that has too much logic in it for just that; some track state, etc). The ultimate interface segregation of the listeners is a good aim, but it doesn't justify all this duplication.

Potential Solution

Unify many if not all of these forwarders and listener collections and so on, and simply handle the interface segregation at the outermost layer.

Impact

There's a great cognitive load in following all these listeners and forwarders and how they all interact and which one you're viewing at any moment because they all look so similar, and there's changes that have been made to one or two but not all, because of the duplication. Simplifying this need not change the outermost interface to this, but would make following and maintaining this code much easier.

Codebase diagram

Problem

New developers that want to work on no-player may find the codebase hard to navigate / not understand fully how different aspects join together.

Potential Solution

Create a high-level diagram for no-player to give an idea of the key links that operate together to make no-player work. We could use https://leanpub.com/visualising-software-architecture or some alternative. I have not looked into this in great detail but I think it would be worthwhile.

Impact

Simplify the onboarding process of new developers.

AndroidMediaPlayerFacade VisibleForTesting

Problem

VisibleForTesting is present in AndroidMediaPlayerFacade. It was added to facilitate testing but is a code smell.

Potential Solution

Remove VisibleForTesting and expose creation of MediaPlayer through a factory or other medium.

Impact

Child classes currently have access to MediaPlayer which we do not want to expose.

Can your player play video for RTSP?

Can your player play video for RTSP?
My video is the type of live stream (RTSP).

Upgrade to Exoplayer v2

TODO or NOT_TODO Forwarders

Problem

We have TODO comments in a lot of the Forwarder classes.

Potential Solution

We should decide whether we want to forward these events or swallow them. If swallowing them then we should provide a reason for this as part of the description of the TODO.

Impact

Provide more information to the client applications by forwarding these additional events.
We supply reasoning as to why we are not forwarding these events, increasing confidence in the codebase.

media player redirect never prepared

Problem

I've noticed the mediaplayer will sometimes not trigger onPrepared or any error/events when the source url is a redirect, using prepare or a debug point just after prepareAsync oddly fixes the issue, should the library forcibly follow redirects? (Or perhaps document this strange behaviour?)

Exoplayer is unaffected.

Potential Solution

To follow redirects

Impact

Would fixes behavioural inconsistencies between media player and exoplayer

AutoValue

Problem

We have quite a few models and we have to generate the hashcode, equals and toString whenever we decide to update them.

Potential Solution

Introduce Auto-Value to have our model classes auto-generated with hashcode, equals and toString whenever we perform a build.

Impact

Less maintenance when adding a new field to a model.

Deprecate getListeners() in favour of add + remove methods

Problem

Having player.getListeners().addMyListener(X) rather than simply player.addMyListener(X) makes for a more complicated interface and more brittle tests. Also applies to remove methods.

Potential Solution

Add the latter method, and ultimately migrate away from the old one, but we could simply deprecate it for now.

Impact

Nice and safe if we just add these and deprecate the old ones. Really good for testing things that use noplayer as a collaborator though.

Break down InfoListener

Problem

com.novoda.noplayer.exoplayer.InfoListener which we inherited from Google's legacy is enormous.

Potential Solution

Breakdown the com.novoda.noplayer.exoplayer.InfoListener into multiple focused interfaces.

Impact

Each class that implements InfoListener needs to implement all methods from the interface or provide them through an abstract class resulting in // no-ops which is a code smell.

Use role from manifest to choose audio

Problem

Currently, if a client wants to select an audio described track they need to have intimate knowledge of the ordering in their manifests to do so. It turns out we don't need to though!

ExoPlayer sets Format.selection flags based on the role. For main this will be 1 and for audio described this will be 0.

Potential Solution

Pull the Format.selection flag when creating the PlayerAudioTrack and set a boolean / enum on the object to highlight the track as audio_described.

Impact

Clients do not need to know the ordering of the manifest.

HeartBeat callback should be consistent and avoid leaking the NoPlayer functionality

Problem

Our HeartbeatCallback is passing through the NoPlayer type whereas our over callbacks use the PlayerState to avoid leaking any extra functionality that the player provides.

interface HeartbeatCallback {
    void onBeat(NoPlayer player);
}

Potential Solution

To lower the type to the PlayerState

Impact

Is a breaking public api change but there is no behaviour change.

onVideoStopped is double called

Problem

As per this, on stop and on release both call onVideoStopped.

Potential Solution

Only emit a state if it is different from the previous state. Similar to how we handle it for the play here.

Impact

Clients should not need to deal with multiple calls to the same state, it could potentially lead to inconsistencies and crashes, especially if we delay patching this.

Assert PlayerView is attached

Problem

We have a number of calls that rely on calling methods on the PlayerView. When calling on the Player, another class that can be null, we assert and throw should this be null.

Potential Solution

Check if PlayerView is null and throw informing clients that they must attach a PlayerView in order to interact with it.

Impact

Consistency in the codebase to how we handle @Nullable. More informative error messaging for a user should they forget to attach a PlayerView rather than a NullPointer.

Avoid creating objects for every second of video

Problem

Strong types are definitely a generally good thing, but in some cases not so appropriate. For VideoPosition we are creating one any time someone queries it. The heartbeat callback is used by many listeners who then query the position, every second. Creating these objects is definitely overkill and could even lead to heap fragmentation, etc. It's quite typical in video work to simply use a 'long seconds' for this kind of thing.

Potential Solution

Use a simple long for position.

Worth considering updating the heartbeat callback method signature to simply pass the position and duration (as longs) at this point, since every listener generally just asks for these two things. The player reference shouldn't really be needed, and if it really is needed then when the listener is created it probably could have been given that reference directly.

Impact

• Changes public API
• Avoids a lot of unnecessary object churn.

Cross protocol redirects

See https://stackoverflow.com/questions/41517440/exoplayer2-how-can-i-make-a-http-301-redirect-work

What should be done if I want to use it inside live wallpaper?

After all, it doesn't have views...
I want to put it and view it in various scaling methods, but for now I've found out only how to do it with the normal MediaPlayer, and it scales it by stretching, which isn't nice:
https://stackoverflow.com/q/50091878/878126

NoPlayer views

Problem

Currently NoPlayerView relies on ExoPlayer view groups coupling NoPlayer to ExoPlayer. See layout for more information.

Potential Solution

We should have NoPlayer implementations of any views that the layout relies on and not be dependent on any views from an underlying Player.

Impact

We won't be coupled to ExoPlayer if we ever drop support for it. We have complete knowledge of the views in our domain.

Update ExoPlayer

Problem

ExoPlayer is now on version 2.5.1 and we are still on 2.4.4.

Potential Solution

Update to the latest version.

Impact

Stability improvements for clients that use the ExoPlayer version of NoPlayer.

Add api to set repeating playback

Problem

We do not currently expose the repeating playback modes of the Media and Exo players

Potential Solution

Expose the apis.

Dash manifest parser

Problem

The ExoPlayerimplementation of a dash manifest parser is restricting when parsing the role tag. Currently it only handles the MAIN tag assuming a particular schemeId is supplied. Ideally we would be able to supply a different role parser based on this schemeId.

Potential Solution

Implement our own dash manifest that is based off of the current implementation, so that we can provide further information to our clients through the PlayerAudioTrack.

We should be able to support additional role tags:

"urn:mpeg:dash:role:2011" for the roles "captions", "subtitle", "main", "alternate", "supplementary", "commentary", and "dub"

Impact

More information for client applications.

Explicitly separate VideoPosition and VideoDuration concepts

Problem

The VideoPosition type is being misused or the method names are inaccurate.

    public VideoPosition minus(VideoPosition other)
    public VideoPosition plus(VideoPosition other)

We can like VideoPosition to a point in time, like "3pm" or "Tuesday". You can get the difference between two points in time, but it should result in an absolute value (non-negative).

VideoPosition is currently being used in two ways:

• a point in time relative to the start of the video
• a length of time

Potential Solution

Introduce a new type for TimeLength or repurpose VideoDuration to represent any length of time (this is the smaller change since at the moment VideoDuration represents the full length of a video).

    public VideoPosition minus(VideoDuration other)
    public VideoPosition plus(VideoDuration other)

Impact

The impact of the types being used indiscriminately is confusion/ambiguity in code. For example:

private static final VideoPosition CREDITS_POSITION_FROM_END_OF_VIDEO = VideoPosition.fromSeconds(60);

private VideoPosition getApproximatePositionOfCreditsGivenFull(VideoDuration videoDuration) {
    VideoPosition endPosition = VideoPosition.fromMillis(videoDuration.inMillis());
    return endPosition.minus(CREDITS_POSITION_FROM_END_OF_VIDEO);
}

I've seen (something like) this in production where the constant isn't accurate. If VideoPosition is a point in the video defined as millis from the start of the video, then using VideoPosition to define a constant of "60 seconds before the end of the video" is impossible without knowing the length of the video.

To do this correctly with the current API we would have:

private static final long CREDITS_POSITION_FROM_END_OF_VIDEO_MILLIS = TimeUnit.SECONDS.toMillis(60);

private VideoPosition getApproximatePositionOfCreditsGivenFull(VideoDuration videoDuration) {
    long creditsPosition = videoDuration.inMillis() - CREDITS_POSITION_FROM_END_OF_VIDEO_MILLIS;
    return VideoPosition.fromMillis(creditsPosition);
}

which is fine, except it's not clear from the API that this is the correct thing to do.

How can i change RepeatMode??

Problem

i want change RepeatMode in playerView. how can i do?
i was try ComponentListener().onRepeatModeChanged(). but cant change.

Provide client-side rules for MediaCodecSelectorFactory

Problem

Currently, the MediaCodecSelectorFactory has a hard-coded check for codec selection.

Solution

Move this check client side.

Impact

We are forcing clients to perform a check that they may not want to support.

Documentation

Problem

We have a lot of public APIs that do not come with any sort of documentation 😭

Potential Solution

Add javadocs for all public APIs with example usages.

Impact

Improve the ease of use of the library.

Update CI release job once `master` in sync with `develop`

Problem

We have recently added the support for snapshot releases as part of #152, but we are missing that code in master. Next time we want to promote the current develop to be part of an official release the CI job (https://ci.novoda.com/job/no-player/) might fail as the release config has changed.

Potential Solution

Ideally we would change the CI job to use the secret properties file (already provided here) similar to what already done for the snapshot job.

Impact

Fixing the issue will allow us to correctly release a new version of the player without issues, and if follow the suggestion above we can start adopting the secrets files to avoid hidden credentials setup and promote explicit setup as part of the build scripts.

utils package should move to internal

Problem

Currently the utils package is top level, we should move to internal to highlight these classes aren't meant for client consumption

Potential Solution

Move to internal/utils

Impact

Clients which are already using these classes will have to reimport the package but they will now be aware these classes are internal

NoPlayerView shutter view not shown on video completion

Problem

The NoPlayerView shutter is not being shown when a video completes.

Potential Solution

This is caused by the ExoPlayerForwarder not taking into account the attached NoPlayerView state changed listeners when initialising, we may need to interact with the forwarder again when attaching the NoPlayerView. This means our NoPlayerView onVideoStopped is ignored.

Impact

• Fixes a bug where the video is shown glitched in the surface view when the video completes due to the shutter not being show.

Rename Player to NoPlayer

Problem

The Player interface conflicts with the ExoPlayer.Player interface.

Potential Solution

Rename the Player interface to NoPlayer.

Impact

Prevent confusion from conflicting names in no-player. It will be clear that the NoPlayer interface belongs to no-player.

Allow the Player to be detached when unprepared

Problem

The NoPlayer can not be detached if it isn't prepared due to subtitles

java.lang.IllegalStateException: Video must be loaded before trying to interact with the player
at com.novoda.noplayer.internal.exoplayer.ExoPlayerFacade.assertVideoLoaded(ExoPlayerFacade.java:197)
at com.novoda.noplayer.internal.exoplayer.ExoPlayerFacade.removeSubtitleRendererOutput(ExoPlayerFacade.java:172)
at com.novoda.noplayer.internal.exoplayer.ExoPlayerTwoImpl.detach(ExoPlayerTwoImpl.java:231)

Potential Solution

Ignoring the removal of subtitles if the internal player is missing.

Impact

• The NoPlayer will be detachable without being prepared.

Technical Data Questionnaire Wiki Page

Problem

We want to show our reasoning as to why someone would want to use this particular library and the challenges we have faced with each release. To this end, we have created a Technical Data Questionnaire. This Questionnaire is cloned from novoda/novoda.

Solution

Here's a template, created from the Gradle Build Properties Plugin, that can be used to create a Data Questionnaire for this repository. The Data Questionnaire should be added to this Repository's Wiki Section.

Template

Overview of the project. What are its technical capabilities compared to the industry standard?

Development of a plugin system for open source applications that require private keys and configs to be ran but these should not be shared in public locations. for example an analytics library that needs a key to login to the system but not all people should access to this key. The project focuses on making the possibility of adding private information as seamless as possible, so that the projects can be used simply but not expose information. This offers features not available currently on the market and although other variations of this technique are possible they do not make the solution quite as easy.

What platforms and tools did you use to make it? (encouraged to namedrop components/languages/databases etc)

Groovy, Gradle for development, Guava, Truth and Findbugs for testing

Describe, in detail, the technical challenges faced (specifically what development required more than routine techniques) and what was engineered to overcome them?

It is really important to discuss the iterative journey to explain at each stage of production (each release) what was engineered.

Write this as if you are explaining the challenges to a technical colleague.

Attempt to write at least four paragraphs that cover these points:

• Technical advances
• How uncertainties were overcome
• Any iterative approaches
• Final solution

Currently to solve the problem of not sharing private keys, developers would cut and paste when they were needed for production and a lot of human interaction was needed to keep these keys private, but still useful. Many repositories would require five or more keys, therefore it required coordination with the build system to make sure each was put in the right place and used at the right time. The gradle build properties plugin automates all of this and allows you to drop in one file, that is read by the gradle system and pulls out the properties or keys and puts them in the right place. This is a technical advancement on any of the current solutions as before it was not centralised or automated meaning keys where in many different places and the chance for security issues was high. With this centralised solution it can allow us to keep all the keys in a secure place and use security credential to access them at the time needed.

A big uncertainty was our knowledge around connecting a remote secure location for the keys to the build system that would be using them. We did not know if we could access or communicate with a secure system in a timely fashion that would allow for the normal build process. This uncertainty was overcome through investigating distribution systems based on maven repositories. With this investigation it was found two way secure communication could happen and therefore we could separate the private keys from the public repository.

When injecting the private keys into a gradle build it has to be done at the correct step. With the creation of this plugin we were unsure in our knowledge of gradle plugins and where we would hook into the build system. This problem was overcome with the development approach of investigating other gradle plugins, this lead us to the Android plugin that also hooks in for other reasons, and we adapted and expanded on this knowledge to allow our key injection to work.

The first approach was to load from a local properties file, take the keys from a separate file, this split apart building and key loading. The second was being able to override at runtime any errors, therefore giving fallback strategies if keys were not available. For example if the properties file was missing a private key, the key could also be set as a system property and we could now fallback from one to the other in a recursive fashion. We then added other fallbacks for secondary files, command line and hard coded values. We then iterated again to allow for the fallback file to be a remote file and take security credentials to open or load it.

The final solution now allows for developers to create open source repositories that use private keys without them needing to be public. The integration is as simple as defining a file, that is not checked into version control but can be in one of multiple places, and then providing fallback strategies if this secure file is not there, for example providing fallback debug keys that can be public and checked into the repository. This simplified any developers use of private keys in a public or private repositories and will likely lead to a decrease in their time spent in this area by a factor of 10x.

Add support for TextureView

Problem

At the moment the integration of ExoPlayer/MediaPlayer is relying on the client to use a SurfaceView as host of the video buffer. This entails a rendering performance problem when the video is hosted in scrollable containers (eg: a RecyclerView), a known issue as described in the official documentation from ExoPlayer:

SurfaceView rendering wasn’t properly synchronized with view animations until Android N. On earlier releases this could result in unwanted effects when a SurfaceView was placed into scrolling container, or when it was subjected to animation. Such effects included the SurfaceView’s contents appearing to lag slightly behind where it should be displayed, and the view turning black when subjected to animation.

Potential Solution

The no-player API is providing facades for both ExoPlayer and MediaPlayer, but one of the callbacks that clients have to provide makes use of android.view.SurfaceHolder, a component that is tightly coupled to the SurfaceView lifecycle.
Both media frameworks provide APIs that accept a Surface in lieu of SurfaceHolder, altho the support in MediaPlayer for those looks to be API 14+ (still below the minsdk of the library afaict).

It should be relatively easy to change the implementation of SurfaceHolderRequester.Callback to deprecate the use of SurfaceHolder and introduce a new callback accepting a plain Surface instead.

Impact

Promoting the use of Surface instead of SurfaceHolder will allow the consumers to choose SurfaceView or TextureView on their side without any repercussion on the player itself.
As suggested above we could try to go for duplication + deprecation as first step, but afaict clients will be affected by the change anyway, and adjusting to the new API is pretty trivial (using SurfaceHolder.getSurface() to retrieve the surface to pass to the callback.

JavaDoc Public APIs

Problem

We have very little to no documentation for our public APIs.

Potential Solution

We should javadoc it, especially considering some method have side effects which will effect the calling of others. Some methods throw exceptions which the clients need to know about and handle.