Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/android subtitle event #3566

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public VideoEventEmitter(ReactContext reactContext) {
private static final String EVENT_VOLUME_CHANGE = "onVolumeChange";
private static final String EVENT_AUDIO_TRACKS = "onAudioTracks";
private static final String EVENT_TEXT_TRACKS = "onTextTracks";

private static final String EVENT_TEXT_TRACK_DATA_CHANGED = "onTextTrackDataChanged";
private static final String EVENT_VIDEO_TRACKS = "onVideoTracks";
private static final String EVENT_ON_RECEIVE_AD_EVENT = "onReceiveAdEvent";

Expand Down Expand Up @@ -86,6 +88,7 @@ public VideoEventEmitter(ReactContext reactContext) {
EVENT_VOLUME_CHANGE,
EVENT_AUDIO_TRACKS,
EVENT_TEXT_TRACKS,
EVENT_TEXT_TRACK_DATA_CHANGED,
EVENT_VIDEO_TRACKS,
EVENT_BANDWIDTH,
EVENT_ON_RECEIVE_AD_EVENT
Expand Down Expand Up @@ -116,6 +119,7 @@ public VideoEventEmitter(ReactContext reactContext) {
EVENT_VOLUME_CHANGE,
EVENT_AUDIO_TRACKS,
EVENT_TEXT_TRACKS,
EVENT_TEXT_TRACK_DATA_CHANGED,
EVENT_VIDEO_TRACKS,
EVENT_BANDWIDTH,
EVENT_ON_RECEIVE_AD_EVENT
Expand Down Expand Up @@ -144,6 +148,7 @@ public VideoEventEmitter(ReactContext reactContext) {
private static final String EVENT_PROP_VIDEO_TRACKS = "videoTracks";
private static final String EVENT_PROP_AUDIO_TRACKS = "audioTracks";
private static final String EVENT_PROP_TEXT_TRACKS = "textTracks";
private static final String EVENT_PROP_TEXT_TRACK_DATA = "subtitleTracks";
private static final String EVENT_PROP_HAS_AUDIO_FOCUS = "hasAudioFocus";
private static final String EVENT_PROP_IS_BUFFERING = "isBuffering";
private static final String EVENT_PROP_PLAYBACK_RATE = "playbackRate";
Expand Down Expand Up @@ -284,6 +289,12 @@ public void textTracks(ArrayList<Track> textTracks){
receiveEvent(EVENT_TEXT_TRACKS, arrayToObject(EVENT_PROP_TEXT_TRACKS, textTracksToArray(textTracks)));
}

public void textTrackDataChanged(String textTrackData){
WritableMap event = Arguments.createMap();
event.putString(EVENT_PROP_TEXT_TRACK_DATA, textTrackData);
receiveEvent(EVENT_TEXT_TRACK_DATA_CHANGED, event);
}

public void videoTracks(ArrayList<VideoTrack> videoTracks){
receiveEvent(EVENT_VIDEO_TRACKS, arrayToObject(EVENT_PROP_VIDEO_TRACKS, videoTracksToArray(videoTracks)));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import androidx.media3.common.TrackGroup;
import androidx.media3.common.TrackSelectionOverride;
import androidx.media3.common.Tracks;
import androidx.media3.common.text.CueGroup;
import androidx.media3.common.util.Util;
import androidx.media3.datasource.DataSource;
import androidx.media3.datasource.DataSpec;
Expand Down Expand Up @@ -1521,6 +1522,13 @@ public void onMetadata(@NonNull Metadata metadata) {
eventEmitter.timedMetadata(metadataArray);
}

public void onCues(CueGroup cueGroup) {
if (!cueGroup.cues.isEmpty() && cueGroup.cues.get(0).text != null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't you think adding following safety check would be a good idea ?

Suggested change
if (!cueGroup.cues.isEmpty() && cueGroup.cues.get(0).text != null) {
if (cueGroup != null && !cueGroup.cues != null && !cueGroup.cues.isEmpty() && cueGroup.cues.get(0).text != null) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I totally agree with adding these safety checks; they’re a good idea. I think there might be a small typo in the suggestion, though. Shouldn’t !cueGroup.cues != null be cueGroup.cues != null?

if (cueGroup != null && cueGroup.cues != null && !cueGroup.cues.isEmpty() && cueGroup.cues.get(0).text != null) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I totally agree with adding these safety checks; they’re a good idea. I think there might be a small typo in the suggestion, though. Shouldn’t !cueGroup.cues != null be cueGroup.cues != null?

if (cueGroup != null && cueGroup.cues != null && !cueGroup.cues.isEmpty() && cueGroup.cues.get(0).text != null) {

And I am not sure it passes the linter πŸ˜…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I tried doing these checks, the suggestion says to simplify to what I had originally? πŸ™ˆ
image

And I am not sure it passes the linter πŸ˜…

which linter? I ran the check-android script and it had changes for an unrelated file; but, not this one?

How would you like me to proceed? 🫑

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't push these since these automated changes are unrelated to this PR.

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK good, let's merge like this so !

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sweet, that sounds awesome 🀩

String subtitleText = cueGroup.cues.get(0).text.toString();
eventEmitter.textTrackDataChanged(subtitleText);
}
}

// ReactExoplayerViewManager public api

public void setSrc(final Uri uri, final long startPositionMs, final long cropStartMs, final long cropEndMs, final String extension, Map<String, String> headers) {
Expand Down
2 changes: 1 addition & 1 deletion docs/pages/component/events.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ Example:

### `onTextTrackDataChanged`

<PlatformsList types={['iOS']} />
<PlatformsList types={['Android', 'iOS']} />

Callback function that is called when new subtitle data is available. It provides the actual subtitle content for the current selected text track, if available (mainly WebVTT).

Expand Down
5 changes: 2 additions & 3 deletions examples/basic/src/VideoPlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,14 @@ class VideoPlayer extends Component {
description: 'another bunny (can be saved)',
uri: 'https://rawgit.com/mediaelement/mediaelement-files/master/big_buck_bunny.mp4',
},
];

srcIosList = [
{
description: 'sintel with subtitles',
uri: 'https://bitmovin-a.akamaihd.net/content/sintel/hls/playlist.m3u8',
},
];

srcIosList = [];

srcAndroidList = [
{
description: 'Another live sample',
Expand Down
Loading