Skip to content

Commit bc7e340

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 b499cf8 commit bc7e340

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;
@@ -84,6 +85,7 @@
8485
import org.schabi.newpipe.R;
8586
import org.schabi.newpipe.databinding.PlayerBinding;
8687
import org.schabi.newpipe.error.ErrorInfo;
88+
import org.schabi.newpipe.error.ErrorPanelHelper;
8789
import org.schabi.newpipe.error.ErrorUtil;
8890
import org.schabi.newpipe.error.UserAction;
8991
import org.schabi.newpipe.extractor.stream.AudioStream;
@@ -107,6 +109,7 @@
107109
import org.schabi.newpipe.player.playback.PlaybackListener;
108110
import org.schabi.newpipe.player.playqueue.PlayQueue;
109111
import org.schabi.newpipe.player.playqueue.PlayQueueItem;
112+
import org.schabi.newpipe.player.playqueue.SinglePlayQueue;
110113
import org.schabi.newpipe.player.resolver.AudioPlaybackResolver;
111114
import org.schabi.newpipe.player.resolver.VideoPlaybackResolver;
112115
import org.schabi.newpipe.player.resolver.VideoPlaybackResolver.SourceType;
@@ -116,8 +119,10 @@
116119
import org.schabi.newpipe.player.ui.PopupPlayerUi;
117120
import org.schabi.newpipe.player.ui.VideoPlayerUi;
118121
import org.schabi.newpipe.util.DependentPreferenceHelper;
122+
import org.schabi.newpipe.util.ExtractorHelper;
119123
import org.schabi.newpipe.util.ListHelper;
120124
import org.schabi.newpipe.util.NavigationHelper;
125+
import org.schabi.newpipe.util.PermissionHelper;
121126
import org.schabi.newpipe.util.image.PicassoHelper;
122127
import org.schabi.newpipe.util.SerializedCache;
123128
import org.schabi.newpipe.util.StreamTypeUtil;
@@ -128,9 +133,11 @@
128133

129134
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers;
130135
import io.reactivex.rxjava3.core.Observable;
136+
import io.reactivex.rxjava3.core.Single;
131137
import io.reactivex.rxjava3.disposables.CompositeDisposable;
132138
import io.reactivex.rxjava3.disposables.Disposable;
133139
import io.reactivex.rxjava3.disposables.SerialDisposable;
140+
import io.reactivex.rxjava3.schedulers.Schedulers;
134141

135142
public final class Player implements PlaybackListener, Listener {
136143
public static final boolean DEBUG = MainActivity.DEBUG;
@@ -158,6 +165,7 @@ public final class Player implements PlaybackListener, Listener {
158165
public static final String PLAY_WHEN_READY = "play_when_ready";
159166
public static final String PLAYER_TYPE = "player_type";
160167
public static final String PLAYER_INTENT_TYPE = "player_intent_type";
168+
public static final String PLAYER_INTENT_DATA = "player_intent_data";
161169

162170
/*//////////////////////////////////////////////////////////////////////////
163171
// Time constants
@@ -242,6 +250,8 @@ public final class Player implements PlaybackListener, Listener {
242250
private final SerialDisposable progressUpdateDisposable = new SerialDisposable();
243251
@NonNull
244252
private final CompositeDisposable databaseUpdateDisposable = new CompositeDisposable();
253+
@NonNull
254+
private final CompositeDisposable streamItemDisposable = new CompositeDisposable();
245255

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

334344
@SuppressWarnings("MethodLength")
335345
public void handleIntent(@NonNull final Intent intent) {
336-
// fail fast if no play queue was provided
337-
final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
338-
if (queueCache == null) {
346+
347+
final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
348+
if (playerIntentType == null) {
339349
return;
340350
}
341-
final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
342-
if (newQueue == null) {
343-
return;
351+
final PlayerType newPlayerType;
352+
// TODO: this should be in the second switch below, but I’m not sure whether I
353+
// can move the initUIs stuff without breaking the setup for edge cases somehow.
354+
switch (playerIntentType) {
355+
case TimestampChange -> {
356+
// TODO: this breaks out of the pattern of asking for the permission before
357+
// sending the PlayerIntent, but I’m not sure yet how to combine the permissions
358+
// with the new enum approach. Maybe it’s better that the player asks anyway?
359+
if (!PermissionHelper.isPopupEnabledElseAsk(context)) {
360+
return;
361+
}
362+
newPlayerType = PlayerType.POPUP;
363+
}
364+
default -> {
365+
newPlayerType = PlayerType.retrieveFromIntent(intent);
366+
}
344367
}
345368

346369
final PlayerType oldPlayerType = playerType;
347-
playerType = PlayerType.retrieveFromIntent(intent);
370+
playerType = newPlayerType;
348371
initUIsForCurrentPlayerType();
349372
// TODO: what does the following comment mean? Is that a relict?
350373
// We need to setup audioOnly before super(), see "sourceOf"
@@ -354,29 +377,66 @@ public void handleIntent(@NonNull final Intent intent) {
354377
videoResolver.setPlaybackQuality(intent.getStringExtra(PLAYBACK_QUALITY));
355378
}
356379

357-
final PlayerIntentType playerIntentType = intent.getParcelableExtra(PLAYER_INTENT_TYPE);
380+
final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
358381

359382
switch (playerIntentType) {
360383
case Enqueue -> {
361384
if (playQueue != null) {
385+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
386+
if (newQueue == null) {
387+
return;
388+
}
362389
playQueue.append(newQueue.getStreams());
363390
}
364391
return;
365392
}
366393
case EnqueueNext -> {
367394
if (playQueue != null) {
395+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
396+
if (newQueue == null) {
397+
return;
398+
}
368399
final int currentIndex = playQueue.getIndex();
369400
playQueue.append(newQueue.getStreams());
370401
playQueue.move(playQueue.size() - 1, currentIndex + 1);
371402
}
372403
return;
373404
}
405+
case TimestampChange -> {
406+
final TimestampChangeData dat = intent.getParcelableExtra(PLAYER_INTENT_DATA);
407+
assert dat != null;
408+
final Single<StreamInfo> single =
409+
ExtractorHelper.getStreamInfo(dat.getServiceId(), dat.getUrl(), false);
410+
streamItemDisposable.add(single.subscribeOn(Schedulers.io())
411+
.observeOn(AndroidSchedulers.mainThread())
412+
.subscribe(info -> {
413+
final PlayQueue newPlayQueue =
414+
new SinglePlayQueue(info, dat.getSeconds() * 1000L);
415+
// TODO: add back the “already playing stream” optimization here
416+
initPlayback(newPlayQueue, playWhenReady);
417+
handleIntentPost(oldPlayerType);
418+
}, throwable -> {
419+
if (DEBUG) {
420+
Log.e(TAG, "Could not play on popup: " + dat.getUrl(), throwable);
421+
}
422+
new AlertDialog.Builder(context)
423+
.setTitle(R.string.player_stream_failure)
424+
.setMessage(
425+
ErrorPanelHelper.Companion.getExceptionDescription(throwable))
426+
.setPositiveButton(R.string.ok, null)
427+
.show();
428+
}));
429+
return;
430+
}
374431
case AllOthers -> {
375432
// fallthrough; TODO: put other intent data in separate cases
376433
}
377434
}
378435

379-
final boolean playWhenReady = intent.getBooleanExtra(PLAY_WHEN_READY, true);
436+
final PlayQueue newQueue = getPlayQueueFromCache(intent);
437+
if (newQueue == null) {
438+
return;
439+
}
380440

381441
// branching parameters for below
382442
final boolean samePlayQueue = playQueue != null && playQueue.equalStreamsAndIndex(newQueue);
@@ -457,6 +517,10 @@ public void handleIntent(@NonNull final Intent intent) {
457517
initPlayback(samePlayQueue ? playQueue : newQueue, playWhenReady);
458518
}
459519

520+
handleIntentPost(oldPlayerType);
521+
}
522+
523+
private void handleIntentPost(final PlayerType oldPlayerType) {
460524
if (oldPlayerType != playerType && playQueue != null) {
461525
// If playerType changes from one to another we should reload the player
462526
// (to disable/enable video stream or to set quality)
@@ -467,6 +531,19 @@ public void handleIntent(@NonNull final Intent intent) {
467531
NavigationHelper.sendPlayerStartedEvent(context);
468532
}
469533

534+
@Nullable
535+
private static PlayQueue getPlayQueueFromCache(@NonNull final Intent intent) {
536+
final String queueCache = intent.getStringExtra(PLAY_QUEUE_KEY);
537+
if (queueCache == null) {
538+
return null;
539+
}
540+
final PlayQueue newQueue = SerializedCache.getInstance().take(queueCache, PlayQueue.class);
541+
if (newQueue == null) {
542+
return null;
543+
}
544+
return newQueue;
545+
}
546+
470547
private void initUIsForCurrentPlayerType() {
471548
if ((UIs.get(MainPlayerUi.class).isPresent() && playerType == PlayerType.MAIN)
472549
|| (UIs.get(PopupPlayerUi.class).isPresent() && playerType == PlayerType.POPUP)) {
@@ -596,6 +673,7 @@ public void destroy() {
596673

597674
databaseUpdateDisposable.clear();
598675
progressUpdateDisposable.set(null);
676+
streamItemDisposable.clear();
599677
cancelLoadingCurrentThumbnail();
600678

601679
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;
@@ -102,6 +103,18 @@ public static <T> Intent getPlayerIntent(@NonNull final Context context,
102103
return intent;
103104
}
104105

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