Skip to content

Commit 44e840c

Browse files
ProfpatschProfpatsch
committed
Player/handleIntent: separate out the timestamp request into enum
Instead of implicitely reconstructing whether the intent was intended (lol) to be a timestamp change, we create a new kind of intent that *only* sets the data we need to switch to a new timestamp. This means that the logic of what to do (opening a popup player) gets moved from `InternalUrlsHandler.playOnPopup` to the `Player.handleIntent` method, we only pass that we want to jump to a new timestamp. Thus, the stream is now loaded *after* sending the intent instead of before sending. This is somewhat messy right now and still does not fix the issue of queue deletion, but from now on the queue logic should get more straightforward to implement. In the end, everything should be a giant switch. Thus we don’t fall-through anymore, but run the post-setup code manually by calling `handeIntentPost` and then returning.
1 parent 1e4d136 commit 44e840c

4 files changed

Lines changed: 122 additions & 83 deletions

File tree

‎app/src/main/java/org/schabi/newpipe/player/Player.java‎

Lines changed: 87 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import static org.schabi.newpipe.util.Localization.assureCorrectAppLanguage;
4848
import static java.util.concurrent.TimeUnit.MILLISECONDS;
4949

50+
import android.app.AlertDialog;
5051
import android.content.BroadcastReceiver;
5152
import android.content.Context;
5253
import android.content.Intent;
@@ -86,6 +87,7 @@
8687
import org.schabi.newpipe.R;
8788
import org.schabi.newpipe.databinding.PlayerBinding;
8889
import org.schabi.newpipe.error.ErrorInfo;
90+
import org.schabi.newpipe.error.ErrorPanelHelper;
8991
import org.schabi.newpipe.error.ErrorUtil;
9092
import org.schabi.newpipe.error.UserAction;
9193
import org.schabi.newpipe.extractor.stream.AudioStream;
@@ -109,6 +111,7 @@
109111
import org.schabi.newpipe.player.playback.PlaybackListener;
110112
import org.schabi.newpipe.player.playqueue.PlayQueue;
111113
import org.schabi.newpipe.player.playqueue.PlayQueueItem;
114+
import org.schabi.newpipe.player.playqueue.SinglePlayQueue;
112115
import org.schabi.newpipe.player.resolver.AudioPlaybackResolver;
113116
import org.schabi.newpipe.player.resolver.VideoPlaybackResolver;
114117
import org.schabi.newpipe.player.resolver.VideoPlaybackResolver.SourceType;
@@ -118,8 +121,10 @@
118121
import org.schabi.newpipe.player.ui.PopupPlayerUi;
119122
import org.schabi.newpipe.player.ui.VideoPlayerUi;
120123
import org.schabi.newpipe.util.DependentPreferenceHelper;
124+
import org.schabi.newpipe.util.ExtractorHelper;
121125
import org.schabi.newpipe.util.ListHelper;
122126
import org.schabi.newpipe.util.NavigationHelper;
127+
import org.schabi.newpipe.util.PermissionHelper;
123128
import org.schabi.newpipe.util.image.PicassoHelper;
124129
import org.schabi.newpipe.util.SerializedCache;
125130
import org.schabi.newpipe.util.StreamTypeUtil;
@@ -130,9 +135,11 @@
130135

131136
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
132137
import io.reactivex.rxjava3.core.Observable;
138+
import io.reactivex.rxjava3.core.Single;
133139
import io.reactivex.rxjava3.disposables.CompositeDisposable;
134140
import io.reactivex.rxjava3.disposables.Disposable;
135141
import io.reactivex.rxjava3.disposables.SerialDisposable;
142+
import io.reactivex.rxjava3.schedulers.Schedulers;
136143

137144
public final class Player implements PlaybackListener, Listener {
138145
public static final boolean DEBUG = MainActivity.DEBUG;
@@ -160,6 +167,7 @@ public final class Player implements PlaybackListener, Listener {
160167
public static final String PLAY_WHEN_READY = "play_when_ready";
161168
public static final String PLAYER_TYPE = "player_type";
162169
public static final String PLAYER_INTENT_TYPE = "player_intent_type";
170+
public static final String PLAYER_INTENT_DATA = "player_intent_data";
163171

164172
/*//////////////////////////////////////////////////////////////////////////
165173
// Time constants
@@ -244,6 +252,8 @@ public final class Player implements PlaybackListener, Listener {
244252
private final SerialDisposable progressUpdateDisposable = new SerialDisposable();
245253
@NonNull
246254
private final CompositeDisposable databaseUpdateDisposable = new CompositeDisposable();
255+
@NonNull
256+
private final CompositeDisposable streamItemDisposable = new CompositeDisposable();
247257

248258
// This is the only listener we need for thumbnail loading, since there is always at most only
249259
// one thumbnail being loaded at a time. This field is also here to maintain a strong reference,
@@ -344,18 +354,31 @@ public int getOverrideResolutionIndex(final List<VideoStream> sortedVideos,
344354

345355
@SuppressWarnings("MethodLength")
346356
public void handleIntent(@NonNull final Intent intent) {
347-
// fail fast if no play queue was provided
348-
final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
349-
if (queueCache == null) {
357+
358+
final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
359+
if (playerIntentType == null) {
350360
return;
351361
}
352-
final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
353-
if (newQueue == null) {
354-
return;
362+
final PlayerType newPlayerType;
363+
// TODO: this should be in the second switch below, but I’m not sure whether I
364+
// can move the initUIs stuff without breaking the setup for edge cases somehow.
365+
switch (playerIntentType) {
366+
case TimestampChange -> {
367+
// TODO: this breaks out of the pattern of asking for the permission before
368+
// sending the PlayerIntent, but I’m not sure yet how to combine the permissions
369+
// with the new enum approach. Maybe it’s better that the player asks anyway?
370+
if (!PermissionHelper.isPopupEnabledElseAsk(context)) {
371+
return;
372+
}
373+
newPlayerType = PlayerType.POPUP;
374+
}
375+
default -> {
376+
newPlayerType = PlayerType.retrieveFromIntent(intent);
377+
}
355378
}
356379

357380
final PlayerType oldPlayerType = playerType;
358-
playerType = PlayerType.retrieveFromIntent(intent);
381+
playerType = newPlayerType;
359382
initUIsForCurrentPlayerType();
360383
// TODO: what does the following comment mean? Is that a relict?
361384
// We need to setup audioOnly before super(), see "sourceOf"
@@ -365,29 +388,66 @@ public void handleIntent(@NonNull final Intent intent) {
365388
videoResolver.setPlaybackQuality(intent.getStringExtra(PLAYBACK_QUALITY));
366389
}
367390

368-
final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
391+
final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
369392

370393
switch (playerIntentType) {
371394
case Enqueue -> {
372395
if (playQueue != null) {
396+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
397+
if (newQueue == null) {
398+
return;
399+
}
373400
playQueue.append(newQueue.getStreams());
374401
}
375402
return;
376403
}
377404
case EnqueueNext -> {
378405
if (playQueue != null) {
406+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
407+
if (newQueue == null) {
408+
return;
409+
}
379410
final int currentIndex = playQueue.getIndex();
380411
playQueue.append(newQueue.getStreams());
381412
playQueue.move(playQueue.size() - 1, currentIndex + 1);
382413
}
383414
return;
384415
}
416+
case TimestampChange -> {
417+
final TimestampChangeData dat = intent.getParcelableExtra(PLAYER_INTENT_DATA);
418+
assert dat != null;
419+
final Single<StreamInfo> single =
420+
ExtractorHelper.getStreamInfo(dat.getServiceId(), dat.getUrl(), false);
421+
streamItemDisposable.add(single.subscribeOn(Schedulers.io())
422+
.observeOn(AndroidSchedulers.mainThread())
423+
.subscribe(info -> {
424+
final PlayQueue newPlayQueue =
425+
new SinglePlayQueue(info, dat.getSeconds() * 1000L);
426+
// TODO: add back the “already playing stream” optimization here
427+
initPlayback(newPlayQueue, playWhenReady);
428+
handleIntentPost(oldPlayerType);
429+
}, throwable -> {
430+
if (DEBUG) {
431+
Log.e(TAG, "Could not play on popup: " + dat.getUrl(), throwable);
432+
}
433+
new AlertDialog.Builder(context)
434+
.setTitle(R.string.player_stream_failure)
435+
.setMessage(
436+
ErrorPanelHelper.Companion.getExceptionDescription(throwable))
437+
.setPositiveButton(R.string.ok, null)
438+
.show();
439+
}));
440+
return;
441+
}
385442
case AllOthers -> {
386443
// fallthrough; TODO: put other intent data in separate cases
387444
}
388445
}
389446

390-
final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
447+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
448+
if (newQueue == null) {
449+
return;
450+
}
391451

392452
// branching parameters for below
393453
final boolean samePlayQueue = playQueue != null && playQueue.equalStreamsAndIndex(newQueue);
@@ -468,6 +528,10 @@ public void handleIntent(@NonNull final Intent intent) {
468528
initPlayback(samePlayQueue ? playQueue : newQueue, playWhenReady);
469529
}
470530

531+
handleIntentPost(oldPlayerType);
532+
}
533+
534+
private void handleIntentPost(final PlayerType oldPlayerType) {
471535
if (oldPlayerType != playerType && playQueue != null) {
472536
// If playerType changes from one to another we should reload the player
473537
// (to disable/enable video stream or to set quality)
@@ -478,6 +542,19 @@ public void handleIntent(@NonNull final Intent intent) {
478542
NavigationHelper.sendPlayerStartedEvent(context);
479543
}
480544

545+
@Nullable
546+
private static PlayQueue getPlayQueueFromCache(@NonNull final Intent intent) {
547+
final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
548+
if (queueCache == null) {
549+
return null;
550+
}
551+
final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
552+
if (newQueue == null) {
553+
return null;
554+
}
555+
return newQueue;
556+
}
557+
481558
private void initUIsForCurrentPlayerType() {
482559
if ((UIs.get(MainPlayerUi.class).isPresent() && playerType == PlayerType.MAIN)
483560
|| (UIs.get(PopupPlayerUi.class).isPresent() && playerType == PlayerType.POPUP)) {
@@ -607,6 +684,7 @@ public void destroy() {
607684

608685
databaseUpdateDisposable.clear();
609686
progressUpdateDisposable.set(null);
687+
streamItemDisposable.clear();
610688
cancelLoadingCurrentThumbnail();
611689

612690
UIs.destroyAll(Object.class); // destroy every UI: obviously every UI extends Object

‎app/src/main/java/org/schabi/newpipe/player/PlayerIntentType.kt‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,16 @@ import kotlinx.parcelize.Parcelize
1111
enum class PlayerIntentType : Parcelable {
1212
Enqueue,
1313
EnqueueNext,
14+
TimestampChange,
1415
AllOthers
1516
}
17+
18+
/**
19+
* A timestamp on the given was clicked and we should switch the playing stream to it.
20+
*/
21+
@Parcelize
22+
data class TimestampChangeData(
23+
val serviceId: Int,
24+
val url: String,
25+
val seconds: Int
26+
) : Parcelable

‎app/src/main/java/org/schabi/newpipe/util/NavigationHelper.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import org.schabi.newpipe.player.PlayerIntentType;
6262
import org.schabi.newpipe.player.PlayerService;
6363
import org.schabi.newpipe.player.PlayerType;
64+
import org.schabi.newpipe.player.TimestampChangeData;
6465
import org.schabi.newpipe.player.helper.PlayerHelper;
6566
import org.schabi.newpipe.player.helper.PlayerHolder;
6667
import org.schabi.newpipe.player.playqueue.PlayQueue;
@@ -103,6 +104,18 @@ public static <T> Intent getPlayerIntent(@NonNull final Context context,
103104
return intent;
104105
}
105106

107+
@NonNull
108+
public static Intent getPlayerTimestampIntent(@NonNull final Context context,
109+
@NonNull final TimestampChangeData
110+
timestampChangeData) {
111+
final Intent intent = new Intent(context, PlayerService.class);
112+
113+
intent.putExtra(Player.PLAYER_INTENT_TYPE, (Parcelable) PlayerIntentType.TimestampChange);
114+
intent.putExtra(Player.PLAYER_INTENT_DATA, timestampChangeData);
115+
116+
return intent;
117+
}
118+
106119
@NonNull
107120
public static <T> Intent getPlayerEnqueueNextIntent(@NonNull final Context context,
108121
@NonNull final Class<T> targetClazz,

‎app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java‎

Lines changed: 11 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,24 @@
11
package org.schabi.newpipe.util.text;
22

33
import android.content.Context;
4-
import android.util.Log;
4+
import android.content.Intent;
55

66
import androidx.annotation.NonNull;
7-
import androidx.appcompat.app.AlertDialog;
7+
import androidx.core.content.ContextCompat;
88

99
import org.schabi.newpipe.MainActivity;
10-
import org.schabi.newpipe.R;
11-
import org.schabi.newpipe.error.ErrorPanelHelper;
1210
import org.schabi.newpipe.extractor.NewPipe;
1311
import org.schabi.newpipe.extractor.StreamingService;
1412
import org.schabi.newpipe.extractor.exceptions.ExtractionException;
1513
import org.schabi.newpipe.extractor.exceptions.ParsingException;
1614
import org.schabi.newpipe.extractor.linkhandler.LinkHandlerFactory;
17-
import org.schabi.newpipe.extractor.stream.StreamInfo;
18-
import org.schabi.newpipe.player.playqueue.PlayQueue;
19-
import org.schabi.newpipe.player.playqueue.SinglePlayQueue;
20-
import org.schabi.newpipe.util.ExtractorHelper;
15+
import org.schabi.newpipe.player.TimestampChangeData;
2116
import org.schabi.newpipe.util.NavigationHelper;
2217

2318
import java.util.regex.Matcher;
2419
import java.util.regex.Pattern;
2520

26-
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
27-
import io.reactivex.rxjava3.core.Single;
2821
import io.reactivex.rxjava3.disposables.CompositeDisposable;
29-
import io.reactivex.rxjava3.schedulers.Schedulers;
3022

3123
public final class InternalUrlsHandler {
3224
private static final String TAG = InternalUrlsHandler.class.getSimpleName();
@@ -36,29 +28,6 @@ public final class InternalUrlsHandler {
3628
private static final Pattern HASHTAG_TIMESTAMP_PATTERN =
3729
Pattern.compile("(.*)#timestamp=(\\d+)");
3830

39-
private InternalUrlsHandler() {
40-
}
41-
42-
/**
43-
* Handle a YouTube timestamp comment URL in NewPipe.
44-
* <p>
45-
* This method will check if the provided url is a YouTube comment description URL ({@code
46-
* https://www.youtube.com/watch?v=}video_id{@code #timestamp=}time_in_seconds). If yes, the
47-
* popup player will be opened when the user will click on the timestamp in the comment,
48-
* at the time and for the video indicated in the timestamp.
49-
*
50-
* @param disposables a field of the Activity/Fragment class that calls this method
51-
* @param context the context to use
52-
* @param url the URL to check if it can be handled
53-
* @return true if the URL can be handled by NewPipe, false if it cannot
54-
*/
55-
public static boolean handleUrlCommentsTimestamp(@NonNull final CompositeDisposable
56-
disposables,
57-
final Context context,
58-
@NonNull final String url) {
59-
return handleUrl(context, url, HASHTAG_TIMESTAMP_PATTERN, disposables);
60-
}
61-
6231
/**
6332
* Handle a YouTube timestamp description URL in NewPipe.
6433
* <p>
@@ -76,27 +45,7 @@ public static boolean handleUrlDescriptionTimestamp(@NonNull final CompositeDisp
7645
disposables,
7746
final Context context,
7847
@NonNull final String url) {
79-
return handleUrl(context, url, AMPERSAND_TIMESTAMP_PATTERN, disposables);
80-
}
81-
82-
/**
83-
* Handle an URL in NewPipe.
84-
* <p>
85-
* This method will check if the provided url can be handled in NewPipe or not. If this is a
86-
* service URL with a timestamp, the popup player will be opened and true will be returned;
87-
* else, false will be returned.
88-
*
89-
* @param context the context to use
90-
* @param url the URL to check if it can be handled
91-
* @param pattern the pattern to use
92-
* @param disposables a field of the Activity/Fragment class that calls this method
93-
* @return true if the URL can be handled by NewPipe, false if it cannot
94-
*/
95-
private static boolean handleUrl(final Context context,
96-
@NonNull final String url,
97-
@NonNull final Pattern pattern,
98-
@NonNull final CompositeDisposable disposables) {
99-
final Matcher matcher = pattern.matcher(url);
48+
final Matcher matcher = AMPERSAND_TIMESTAMP_PATTERN.matcher(url);
10049
if (!matcher.matches()) {
10150
return false;
10251
}
@@ -153,25 +102,13 @@ public static boolean playOnPopup(final Context context,
153102
return false;
154103
}
155104

156-
final Single<StreamInfo> single =
157-
ExtractorHelper.getStreamInfo(service.getServiceId(), cleanUrl, false);
158-
disposables.add(single.subscribeOn(Schedulers.io())
159-
.observeOn(AndroidSchedulers.mainThread())
160-
.subscribe(info -> {
161-
final PlayQueue playQueue =
162-
new SinglePlayQueue(info, seconds * 1000L);
163-
NavigationHelper.playOnPopupPlayer(context, playQueue, false);
164-
}, throwable -> {
165-
if (DEBUG) {
166-
Log.e(TAG, "Could not play on popup: " + url, throwable);
167-
}
168-
new AlertDialog.Builder(context)
169-
.setTitle(R.string.player_stream_failure)
170-
.setMessage(
171-
ErrorPanelHelper.Companion.getExceptionDescription(throwable))
172-
.setPositiveButton(R.string.ok, null)
173-
.show();
174-
}));
105+
final Intent intent = NavigationHelper.getPlayerTimestampIntent(context,
106+
new TimestampChangeData(
107+
service.getServiceId(),
108+
cleanUrl,
109+
seconds
110+
));
111+
ContextCompat.startForegroundService(context, intent);
175112
return true;
176113
}
177114
}

0 commit comments

Comments
 (0)