Skip to content

Commit

Permalink
Don't select trick-play tracks by default
Browse files Browse the repository at this point in the history 
Issue: #6054
Issue: #474
PiperOrigin-RevId: 306504362
  • Loading branch information
@ojw28 @icbaker
ojw28 authored and icbaker committed Apr 15, 2020
1 parent a99288a commit 6cff8a6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
8 changes: 6 additions & 2 deletions library/common/src/main/java/com/google/android/exoplayer2/C.java
View file
Expand Up @@ -998,7 +998,8 @@ private C() {}
* #ROLE_FLAG_DUB}, {@link #ROLE_FLAG_EMERGENCY}, {@link #ROLE_FLAG_CAPTION}, {@link
* #ROLE_FLAG_SUBTITLE}, {@link #ROLE_FLAG_SIGN}, {@link #ROLE_FLAG_DESCRIBES_VIDEO}, {@link
* #ROLE_FLAG_DESCRIBES_MUSIC_AND_SOUND}, {@link #ROLE_FLAG_ENHANCED_DIALOG_INTELLIGIBILITY},
* {@link #ROLE_FLAG_TRANSCRIBES_DIALOG} and {@link #ROLE_FLAG_EASY_TO_READ}.
* {@link #ROLE_FLAG_TRANSCRIBES_DIALOG}, {@link #ROLE_FLAG_EASY_TO_READ} and {@link
* #ROLE_FLAG_TRICK_PLAY}.
*/
@Documented
@Retention(RetentionPolicy.SOURCE)
Expand All @@ -1018,7 +1019,8 @@ private C() {}
ROLE_FLAG_DESCRIBES_MUSIC_AND_SOUND,
ROLE_FLAG_ENHANCED_DIALOG_INTELLIGIBILITY,
ROLE_FLAG_TRANSCRIBES_DIALOG,
ROLE_FLAG_EASY_TO_READ
ROLE_FLAG_EASY_TO_READ,
ROLE_FLAG_TRICK_PLAY
})
public @interface RoleFlags {}
/** Indicates a main track. */
Expand Down Expand Up @@ -1064,6 +1066,8 @@ private C() {}
public static final int ROLE_FLAG_TRANSCRIBES_DIALOG = 1 << 12;
/** Indicates the track contains a text that has been edited for ease of reading. */
public static final int ROLE_FLAG_EASY_TO_READ = 1 << 13;
/** Indicates the track is intended for trick play. */
public static final int ROLE_FLAG_TRICK_PLAY = 1 << 14;

/**
* Converts a time in microseconds to the corresponding time in milliseconds, preserving
Expand Down
10 changes: 9 additions & 1 deletion ...core/src/main/java/com/google/android/exoplayer2/trackselection/DefaultTrackSelector.java
View file
Expand Up @@ -1888,6 +1888,10 @@ private static boolean isSupportedAdaptiveVideoTrack(
int maxVideoHeight,
int maxVideoFrameRate,
int maxVideoBitrate) {
if ((format.roleFlags & C.ROLE_FLAG_TRICK_PLAY) != 0) {
// Ignore trick-play tracks for now.
return false;
}
return isSupported(formatSupport, false)

This comment has been minimized.

Sign in to view

Copy link
@stevemayhew

stevemayhew May 4, 2020

Contributor

@ojw28 Not sure the vision for this, with this change it is not possible to (simply) override in order to get the trickplay tracks into an AdaptiveTrackSelection set.

I think the most strait forward way is to run a complete track selection when switching into/out of trick-play mode. That would allow selecting appropriate renderers (like disabling sounds at highest speeds). So maybe there is a TrackSelectionParameters that sets trick-play mode[s].

Your thoughts?

This comment has been minimized.

Sign in to view

Copy link
@ojw28

ojw28 May 4, 2020

Author Contributor

with this change it is not possible to (simply) override in order to get the trick-play tracks into an AdaptiveTrackSelection set.

What type of overriding are you referring to? It is possible to pass a track selection override to the selector (using setSelectionOverride) that includes trick-play tracks. This can be used today to re-select the appropriate tracks when entering/exiting trick-play mode, in addition to any other required actions such as disabling audio.

I assume you're referring to extending the class. In which case yes, this is not currently particularly easy.

Since the required actions for trick-play seem to be common (enable trick-play tracks, disable audio, tweak how seeking is performed to ensure that at least one key-frame is rendered before the next seek is processed, possibly tweak how buffering works), I think the slightly longer term vision is to have an API like 'Player.setTrickPlayEnabled(boolean)' that will cause all of the right things to happen.

This comment has been minimized.

Sign in to view

Copy link
@stevemayhew

stevemayhew May 4, 2020

Contributor

Selection overrides could work... We have other bits that are doing selection overrides for things (explicit selection of audio tracks, captions, etc) for other purposes. Would you see the selection override as a long term solution or just for development?

This is probably not the best place to discuss the design, can we share a doc on Google again?

This comment has been minimized.

Sign in to view

Copy link
@ojw28

ojw28 May 5, 2020 •
edited

Author Contributor

I don't think selection overrides will be the final solution. I think we need something more like what I described above. I haven't had a chance to think about it properly yet, but it's on my list for some time in Q2.

You're welcome to make a proposal in the meantime!
&& ((formatSupport & requiredAdaptiveSupport) != 0)
&& (mimeType == null || Util.areEqual(format.sampleMimeType, mimeType))
Expand All @@ -1911,9 +1915,13 @@ private static TrackSelection.Definition selectFixedVideoTrack(
params.viewportWidth, params.viewportHeight, params.viewportOrientationMayChange);
@Capabilities int[] trackFormatSupport = formatSupports[groupIndex];
for (int trackIndex = 0; trackIndex < trackGroup.length; trackIndex++) {
Format format = trackGroup.getFormat(trackIndex);
if ((format.roleFlags & C.ROLE_FLAG_TRICK_PLAY) != 0) {
// Ignore trick-play tracks for now.
continue;
}
if (isSupported(trackFormatSupport[trackIndex],
params.exceedRendererCapabilitiesIfNecessary)) {
Format format = trackGroup.getFormat(trackIndex);
boolean isWithinConstraints =
selectedTrackIndices.contains(trackIndex)
&& (format.width == Format.NO_VALUE || format.width <= params.maxVideoWidth)
Expand Down

0 comments on commit 6cff8a6

Please sign in to comment.