From 94dd942caa49320184401fee56fead3e62321063 Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Tue, 10 Jun 2025 02:53:42 +0100 Subject: [PATCH 01/10] ListHelper.getFilteredAudioStreams short circuit on empty list --- app/src/main/java/org/schabi/newpipe/util/ListHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java index 409fcb30cbd..4bf84d64524 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -293,7 +293,7 @@ public static boolean isHighResolutionSelected(final String selectedResolution, public static List getFilteredAudioStreams( @NonNull final Context context, @Nullable final List audioStreams) { - if (audioStreams == null) { + if (audioStreams == null || audioStreams.isEmpty()) { return Collections.emptyList(); } From 7b121f2d7f00a1791dc6a6c44270dfaa6c00e43f Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Thu, 3 Jul 2025 01:16:05 +0100 Subject: [PATCH 02/10] Allow comment suppression in checkstyle.xml --- checkstyle/checkstyle.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/checkstyle/checkstyle.xml b/checkstyle/checkstyle.xml index ee091fa9f44..ff2554f77de 100644 --- a/checkstyle/checkstyle.xml +++ b/checkstyle/checkstyle.xml @@ -70,6 +70,8 @@ + + From 4e32cbcdf546545e2db5316f0699fe1171236790 Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Thu, 3 Jul 2025 03:47:39 +0100 Subject: [PATCH 03/10] Setup ExtractorLogger in MainActivity Improve logging in InfoCache.java and elsewhere Log state name in some methods Minor refactoring --- .../java/org/schabi/newpipe/MainActivity.java | 25 ++++++++++++ .../org/schabi/newpipe/player/Player.java | 39 +++++++++++++++++-- .../NonUriHlsDataSourceFactory.java | 2 +- .../mediasession/MediaSessionPlayerUi.java | 2 +- .../player/playback/MediaSourceManager.java | 35 +++++++++++++++-- .../newpipe/player/playqueue/PlayQueue.java | 7 +--- .../org/schabi/newpipe/util/InfoCache.java | 30 ++++++++++---- .../org/schabi/newpipe/util/ListHelper.java | 3 ++ .../schabi/newpipe/util/SparseItemUtil.java | 2 +- 9 files changed, 121 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index 8dac39682fc..c6c5e9b8e24 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -69,6 +69,8 @@ import org.schabi.newpipe.extractor.comments.CommentsInfoItem; import org.schabi.newpipe.extractor.exceptions.ExtractionException; import org.schabi.newpipe.extractor.services.peertube.PeertubeInstance; +import org.schabi.newpipe.extractor.utils.ExtractorLogger; +import org.schabi.newpipe.extractor.utils.Logger; import org.schabi.newpipe.fragments.BackPressable; import org.schabi.newpipe.fragments.MainFragment; import org.schabi.newpipe.fragments.detail.VideoDetailFragment; @@ -137,6 +139,29 @@ public class MainActivity extends AppCompatActivity { @Override protected void onCreate(final Bundle savedInstanceState) { if (DEBUG) { + // Override Extractor to print to Logcat + ExtractorLogger.setLogger(new Logger() { + @Override + public void debug(final String tag, final String message) { + Log.d(tag, message); + } + + @Override + public void warn(final String tag, final String message) { + Log.w(tag, message); + } + + @Override + public void error(final String tag, final String message) { + Log.e(tag, message); + } + + @Override + public void error(final String tag, final String message, final Throwable t) { + Log.e(tag, message, t); + } + + }); Log.d(TAG, "onCreate() called with: " + "savedInstanceState = [" + savedInstanceState + "]"); } diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java index e18ead89992..be3cea5ff00 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -732,7 +732,9 @@ private void setRecovery(final int queuePos, final long windowPos) { } if (DEBUG) { - Log.d(TAG, "Setting recovery, queue: " + queuePos + ", pos: " + windowPos); + final var currentTitle = currentItem != null ? currentItem.getTitle() : ""; + Log.d(TAG, "Setting recovery, queue: " + + queuePos + "[" + currentTitle + "], pos: " + windowPos); } playQueue.setRecovery(queuePos, windowPos); } @@ -1071,11 +1073,39 @@ public void onPlayWhenReadyChanged(final boolean playWhenReady, final int reason updatePlaybackState(playWhenReady, playbackState); } + public static String exoplayerStateToString(final int playbackState) { + return switch (playbackState) { + case com.google.android.exoplayer2.Player.STATE_IDLE -> // 1 + "STATE_IDLE"; + case com.google.android.exoplayer2.Player.STATE_BUFFERING -> // 2 + "STATE_BUFFERING"; + case com.google.android.exoplayer2.Player.STATE_READY -> //3 + "STATE_READY"; + case com.google.android.exoplayer2.Player.STATE_ENDED -> // 4 + "STATE_ENDED"; + default -> + throw new IllegalArgumentException("Unknown playback state " + playbackState); + }; + } + + public static String stateToString(final int state) { + return switch (state) { + case STATE_PREFLIGHT -> "STATE_PREFLIGHT"; + case STATE_BLOCKED -> "STATE_BLOCKED"; + case STATE_PLAYING -> "STATE_PLAYING"; + case STATE_BUFFERING -> "STATE_BUFFERING"; + case STATE_PAUSED -> "STATE_PAUSED"; + case STATE_PAUSED_SEEK -> "STATE_PAUSED_SEEK"; + case STATE_COMPLETED -> "STATE_COMPLETED"; + default -> throw new IllegalArgumentException("Unknown playback state " + state); + }; + } + @Override public void onPlaybackStateChanged(final int playbackState) { if (DEBUG) { Log.d(TAG, "ExoPlayer - onPlaybackStateChanged() called with: " - + "playbackState = [" + playbackState + "]"); + + "playbackState = [" + exoplayerStateToString(playbackState) + "]"); } updatePlaybackState(getPlayWhenReady(), playbackState); } @@ -1162,7 +1192,8 @@ public void onPlaybackUnblock(final MediaSource mediaSource) { public void changeState(final int state) { if (DEBUG) { - Log.d(TAG, "changeState() called with: state = [" + state + "]"); + Log.d(TAG, + "changeState() called with: state = [" + stateToString(state) + "]"); } currentState = state; switch (state) { @@ -1882,7 +1913,7 @@ private void saveStreamProgressState(final long progressMillis) { .observeOn(AndroidSchedulers.mainThread()) .doOnError(e -> { if (DEBUG) { - e.printStackTrace(); + Log.e(TAG, "Error saving stream state", e); } }) .onErrorComplete() diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/NonUriHlsDataSourceFactory.java b/app/src/main/java/org/schabi/newpipe/player/datasource/NonUriHlsDataSourceFactory.java index 676443a9c78..90ed3af1f4d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/datasource/NonUriHlsDataSourceFactory.java +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/NonUriHlsDataSourceFactory.java @@ -112,7 +112,7 @@ private NonUriHlsDataSourceFactory(@NonNull final DataSource.Factory dataSourceF *

* *

- * This change allow playback of non-URI HLS contents, when the manifest is not a master + * This change allows playback of non-URI HLS contents, when the manifest is not a master * manifest/playlist (otherwise, endless loops should be encountered because the * {@link DataSource}s created for media playlists should use the master playlist response * instead). diff --git a/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java b/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java index fe884834bc3..abc3c1844ad 100644 --- a/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java +++ b/app/src/main/java/org/schabi/newpipe/player/mediasession/MediaSessionPlayerUi.java @@ -36,7 +36,7 @@ public class MediaSessionPlayerUi extends PlayerUi implements SharedPreferences.OnSharedPreferenceChangeListener { - private static final String TAG = "MediaSessUi"; + private static final String TAG = MediaSessionPlayerUi.class.getSimpleName(); @NonNull private final MediaSessionCompat mediaSession; diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java index 5cffc7f62f4..dde4cd0a6ef 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java @@ -9,6 +9,7 @@ import org.reactivestreams.Subscriber; import org.reactivestreams.Subscription; +import org.schabi.newpipe.extractor.StreamingServiceId; import org.schabi.newpipe.extractor.exceptions.ExtractionException; import org.schabi.newpipe.player.mediaitem.MediaItemTag; import org.schabi.newpipe.player.mediasource.FailedMediaSource; @@ -114,6 +115,9 @@ public class MediaSourceManager { @NonNull private final CompositeDisposable loaderReactor; + /** + * The items that are currently being loaded. + */ @NonNull private final Set loadingItems; @@ -146,6 +150,20 @@ private MediaSourceManager(@NonNull final PlaybackListener listener, + " ms] for them to be useful."); } + if (DEBUG) { + final var currentItem = playQueue.getItem(); + + if (currentItem != null) { + Log.d(TAG, "Creating MediaSourceManager[" + + StreamingServiceId.nameFromId(currentItem.getServiceId()) + + " currentIndex=" + playQueue.getIndex() + + " currentTitle=" + currentItem.getTitle()); + } else { + Log.d(TAG, + "Creating MediaSourceManager[currentIndex=" + playQueue.getIndex() + "]"); + } + } + this.playbackListener = listener; this.playQueue = playQueue; @@ -180,13 +198,15 @@ private MediaSourceManager(@NonNull final PlaybackListener listener, */ public void dispose() { if (DEBUG) { - Log.d(TAG, "close() called."); + Log.d(TAG, "dispose() called."); } debouncedSignal.onComplete(); debouncedLoader.dispose(); playQueueReactor.cancel(); + //TODO: Why not clear here? + //TODO: Also why not clear loadingItems here? loaderReactor.dispose(); } @@ -427,6 +447,11 @@ private Single getLoadedMediaSource(@NonNull final PlayQueue MediaItemTag.from(source.getMediaItem()) .map(tag -> { final int serviceId = streamInfo.getServiceId(); + // CHECKSTYLE:OFF + // TODO: So this expiration is false. Expiration starts from when stream is extracted + // But here it is giving it fresh expiration as though the stream info is new, but it's not + // So cache expiration is not 1-1 with stream expiration, but is it supposed to be? + // CHECKSTYLE:ON final long expiration = System.currentTimeMillis() + getCacheExpirationMillis(serviceId); return new LoadedMediaSource(source, tag, stream, @@ -483,7 +508,7 @@ private void onMediaSourceReceived(@NonNull final PlayQueueItem item, * readiness or playlist desynchronization. *

* If the given {@link PlayQueueItem} is currently being played and is already loaded, - * then correction is not only needed if the playlist is desynchronized. Otherwise, the + * then correction is only needed if the playlist is desynchronized. Otherwise, the * check depends on the status (e.g. expiration or placeholder) of the * {@link ManagedMediaSource}. *

@@ -530,10 +555,12 @@ private void maybeRenewCurrentIndex() { } private void maybeClearLoaders() { + final var currentItem = playQueue.getItem(); if (DEBUG) { - Log.d(TAG, "MediaSource - maybeClearLoaders() called."); + final var url = currentItem != null ? currentItem.getUrl() : ""; + Log.d(TAG, "MediaSource - maybeClearLoaders() called. currentItem: " + url); } - if (!loadingItems.contains(playQueue.getItem()) + if (!loadingItems.contains(currentItem) && loaderReactor.size() > MAXIMUM_LOADER_SIZE) { loaderReactor.clear(); loadingItems.clear(); diff --git a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java index 8a66dc2ffac..62104747e6b 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java +++ b/app/src/main/java/org/schabi/newpipe/player/playqueue/PlayQueue.java @@ -552,11 +552,8 @@ public boolean equalStreams(@Nullable final PlayQueue other) { } public boolean equalStreamsAndIndex(@Nullable final PlayQueue other) { - if (equalStreams(other)) { - //noinspection ConstantConditions - return other.getIndex() == getIndex(); //NOSONAR: other is not null - } - return false; + //noinspection ConstantConditions + return equalStreams(other) && other.getIndex() == getIndex(); //NOSONAR: other is not null } public boolean isDisposed() { diff --git a/app/src/main/java/org/schabi/newpipe/util/InfoCache.java b/app/src/main/java/org/schabi/newpipe/util/InfoCache.java index b9c91f8a5b0..7172f4a18ef 100644 --- a/app/src/main/java/org/schabi/newpipe/util/InfoCache.java +++ b/app/src/main/java/org/schabi/newpipe/util/InfoCache.java @@ -27,8 +27,11 @@ import org.schabi.newpipe.MainActivity; import org.schabi.newpipe.extractor.Info; +import org.schabi.newpipe.extractor.StreamingServiceId; -import java.util.Map; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneId; public final class InfoCache { private final String TAG = getClass().getSimpleName(); @@ -71,23 +74,24 @@ private static String keyOf(final int serviceId, } private static void removeStaleCache() { - for (final Map.Entry entry : InfoCache.LRU_CACHE.snapshot().entrySet()) { + for (final var entry : LRU_CACHE.snapshot().entrySet()) { final CacheData data = entry.getValue(); if (data != null && data.isExpired()) { - InfoCache.LRU_CACHE.remove(entry.getKey()); + LRU_CACHE.remove(entry.getKey()); } } } @Nullable private static Info getInfo(@NonNull final String key) { - final CacheData data = InfoCache.LRU_CACHE.get(key); + final CacheData data = LRU_CACHE.get(key); if (data == null) { return null; } if (data.isExpired()) { - InfoCache.LRU_CACHE.remove(key); + // TODO: oh so we do check expiry on retrieval! + LRU_CACHE.remove(key); return null; } @@ -100,22 +104,32 @@ public Info getFromKey(final int serviceId, @NonNull final Type cacheType) { if (DEBUG) { Log.d(TAG, "getFromKey() called with: " - + "serviceId = [" + serviceId + "], url = [" + url + "]"); + + StreamingServiceId.nameFromId(serviceId) + "[" + url + "]"); } synchronized (LRU_CACHE) { return getInfo(keyOf(serviceId, url, cacheType)); } } + @SuppressWarnings("checkStyle:linelength") public void putInfo(final int serviceId, @NonNull final String url, @NonNull final Info info, @NonNull final Type cacheType) { + final long expirationMillis = ServiceHelper.getCacheExpirationMillis(info.getServiceId()); + // CHECKSTYLE:OFF + // TODO: Expired items get refreshed by being put again; they are not removed when they expire + // CHECKSTYLE:ON if (DEBUG) { - Log.d(TAG, "putInfo() called with: info = [" + info + "]"); + final var expiryDateInstant = Instant.now().plusMillis(expirationMillis); + final var expiryDate = LocalDateTime.ofInstant(expiryDateInstant, + ZoneId.systemDefault()); + Log.d(TAG, "putInfo(): add to cache " + StreamingServiceId.nameFromId(serviceId) + " " + + cacheType.name() + + " expires on " + expiryDate + + " " + info); } - final long expirationMillis = ServiceHelper.getCacheExpirationMillis(info.getServiceId()); synchronized (LRU_CACHE) { final CacheData data = new CacheData(info, expirationMillis); LRU_CACHE.put(keyOf(serviceId, url, cacheType), data); diff --git a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java index 4bf84d64524..6b921a9daa6 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -302,6 +302,9 @@ public static List getFilteredAudioStreams( final Comparator cmp = getAudioFormatComparator(context); for (final AudioStream stream : audioStreams) { + // TODO: this doesn't add HLS OPUS streams, but soundcloud has that. + // Meaning it never actually plays the OPUS soundcloud streams, only + // progressive and hls mp3 if (stream.getDeliveryMethod() == DeliveryMethod.TORRENT || (stream.getDeliveryMethod() == DeliveryMethod.HLS && stream.getFormat() == MediaFormat.OPUS)) { diff --git a/app/src/main/java/org/schabi/newpipe/util/SparseItemUtil.java b/app/src/main/java/org/schabi/newpipe/util/SparseItemUtil.java index 05f26f1784a..e73898cd7a9 100644 --- a/app/src/main/java/org/schabi/newpipe/util/SparseItemUtil.java +++ b/app/src/main/java/org/schabi/newpipe/util/SparseItemUtil.java @@ -32,7 +32,7 @@ private SparseItemUtil() { } /** - * Use this to certainly obtain an single play queue with all of the data filled in when the + * Use this to certainly obtain a single play queue with all of the data filled in when the * stream info item you are handling might be sparse, e.g. because it was fetched via a {@link * org.schabi.newpipe.extractor.feed.FeedExtractor}. FeedExtractors provide a fast and * lightweight method to fetch info, but the info might be incomplete (see From 0f273363d97a1910acad0bf369a5ba158b5b51cb Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Thu, 3 Jul 2025 05:21:34 +0100 Subject: [PATCH 04/10] Add LoggingHttpDataSource to log http requests sent by ExoPlayer for non YouTube streams --- .../datasource/LoggingHttpDataSource.java | 118 ++++++++++++++++++ .../player/helper/PlayerDataSource.java | 32 +++-- .../player/resolver/PlaybackResolver.java | 3 +- 3 files changed, 140 insertions(+), 13 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java new file mode 100644 index 00000000000..3d5f1e81594 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java @@ -0,0 +1,118 @@ +package org.schabi.newpipe.player.datasource; + +import android.util.Log; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import com.google.android.exoplayer2.upstream.DataSpec; +import com.google.android.exoplayer2.upstream.DefaultHttpDataSource; +import com.google.android.exoplayer2.upstream.HttpDataSource; +import com.google.android.exoplayer2.upstream.TransferListener; + +import org.schabi.newpipe.DownloaderImpl; + +import java.nio.charset.StandardCharsets; +import java.util.Map; + +public class LoggingHttpDataSource extends DefaultHttpDataSource { + + public final String TAG = getClass().getSimpleName() + "@" + hashCode(); + + public LoggingHttpDataSource() { } + + public LoggingHttpDataSource(@Nullable final String userAgent, + final int connectTimeoutMillis, + final int readTimeoutMillis, + final boolean allowCrossProtocolRedirects, + @Nullable final RequestProperties defaultRequestProperties) { + super(userAgent, + connectTimeoutMillis, + readTimeoutMillis, + allowCrossProtocolRedirects, + defaultRequestProperties); + } + + + @Override + public long open(final DataSpec dataSpec) throws HttpDataSourceException { + Log.d(TAG, "Request URL: " + dataSpec.uri); + try { + return super.open(dataSpec); + } catch (final HttpDataSource.InvalidResponseCodeException e) { + Log.e(TAG, "HTTP error for URL: " + dataSpec.uri); + Log.e(TAG, "Response code: " + e.responseCode); + Log.e(TAG, "Headers: " + e.headerFields); + Log.e(TAG, "Body: " + new String(e.responseBody, StandardCharsets.UTF_8)); + throw e; + } + } + + @SuppressWarnings("checkstyle:hiddenField") + public static class Factory implements HttpDataSource.Factory { + + final RequestProperties defaultRequestProperties; + + @Nullable + TransferListener transferListener; + @Nullable + String userAgent; + int connectTimeoutMs; + int readTimeoutMs; + boolean allowCrossProtocolRedirects; + + public Factory() { + defaultRequestProperties = new RequestProperties(); + connectTimeoutMs = DEFAULT_CONNECT_TIMEOUT_MILLIS; + readTimeoutMs = DEFAULT_READ_TIMEOUT_MILLIS; + userAgent = DownloaderImpl.USER_AGENT; + } + + @NonNull + @Override + public HttpDataSource createDataSource() { + final var dataSource = new LoggingHttpDataSource(userAgent, + connectTimeoutMs, + readTimeoutMs, + allowCrossProtocolRedirects, + defaultRequestProperties); + if (transferListener != null) { + dataSource.addTransferListener(transferListener); + } + return dataSource; + } + + @NonNull + @Override + public Factory setDefaultRequestProperties( + @NonNull final Map defaultRequestProperties) { + this.defaultRequestProperties.clearAndSet(defaultRequestProperties); + return this; + } + + public Factory setUserAgent(@Nullable final String userAgent) { + this.userAgent = userAgent; + return this; + } + + public Factory setConnectTimeoutMs(final int connectTimeoutMs) { + this.connectTimeoutMs = connectTimeoutMs; + return this; + } + + public Factory setReadTimeoutMs(final int readTimeoutMs) { + this.readTimeoutMs = readTimeoutMs; + return this; + } + + public Factory setAllowCrossProtocolRedirects(final boolean allowCrossProtocolRedirects) { + this.allowCrossProtocolRedirects = allowCrossProtocolRedirects; + return this; + } + + public Factory setTransferListener(@Nullable final TransferListener transferListener) { + this.transferListener = transferListener; + return this; + } + } +} diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java index 506b643fed4..eeee9bac563 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java @@ -5,7 +5,7 @@ import android.content.Context; import android.util.Log; -import androidx.annotation.Nullable; +import androidx.annotation.NonNull; import com.google.android.exoplayer2.database.StandaloneDatabaseProvider; import com.google.android.exoplayer2.source.ProgressiveMediaSource; @@ -18,15 +18,15 @@ import com.google.android.exoplayer2.source.smoothstreaming.SsMediaSource; import com.google.android.exoplayer2.upstream.DataSource; import com.google.android.exoplayer2.upstream.DefaultDataSource; -import com.google.android.exoplayer2.upstream.DefaultHttpDataSource; import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.upstream.cache.LeastRecentlyUsedCacheEvictor; import com.google.android.exoplayer2.upstream.cache.SimpleCache; -import org.schabi.newpipe.DownloaderImpl; +import org.jetbrains.annotations.Contract; import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubeOtfDashManifestCreator; import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubePostLiveStreamDvrDashManifestCreator; import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubeProgressiveDashManifestCreator; +import org.schabi.newpipe.player.datasource.LoggingHttpDataSource; import org.schabi.newpipe.player.datasource.NonUriHlsDataSourceFactory; import org.schabi.newpipe.player.datasource.YoutubeHttpDataSource; @@ -78,27 +78,32 @@ public class PlayerDataSource { private final CacheFactory ytProgressiveDashCacheDataSourceFactory; + private final Context context; + private final TransferListener transferListener; + + public PlayerDataSource(final Context context, final TransferListener transferListener) { - + this.context = context; + this.transferListener = transferListener; progressiveLoadIntervalBytes = PlayerHelper.getProgressiveLoadIntervalBytes(context); // make sure the static cache was created: needed by CacheFactories below instantiateCacheIfNeeded(context); - // generic data source factories use DefaultHttpDataSource.Factory + // generic data source factories use LoggingHttpDataSource.Factory, which is a wrapper + // around DefaultHttpDataSource cachelessDataSourceFactory = new DefaultDataSource.Factory(context, - new DefaultHttpDataSource.Factory().setUserAgent(DownloaderImpl.USER_AGENT)) + new LoggingHttpDataSource.Factory()) .setTransferListener(transferListener); - cacheDataSourceFactory = new CacheFactory(context, transferListener, cache, - new DefaultHttpDataSource.Factory().setUserAgent(DownloaderImpl.USER_AGENT)); + cacheDataSourceFactory = createCacheDataSourceFactory(new LoggingHttpDataSource.Factory()); // YouTube-specific data source factories use getYoutubeHttpDataSourceFactory() - ytHlsCacheDataSourceFactory = new CacheFactory(context, transferListener, cache, + ytHlsCacheDataSourceFactory = createCacheDataSourceFactory( getYoutubeHttpDataSourceFactory(false, false)); - ytDashCacheDataSourceFactory = new CacheFactory(context, transferListener, cache, + ytDashCacheDataSourceFactory = createCacheDataSourceFactory( getYoutubeHttpDataSourceFactory(true, true)); - ytProgressiveDashCacheDataSourceFactory = new CacheFactory(context, transferListener, cache, + ytProgressiveDashCacheDataSourceFactory = createCacheDataSourceFactory( getYoutubeHttpDataSourceFactory(false, true)); // set the maximum size to manifest creators @@ -108,6 +113,11 @@ public PlayerDataSource(final Context context, MAX_MANIFEST_CACHE_SIZE); } + @NonNull + @Contract(value = "_ -> new", pure = true) + private CacheFactory createCacheDataSourceFactory(final DataSource.Factory wrappedFactory) { + return new CacheFactory(context, transferListener, cache, wrappedFactory); + } //region Live media source factories public SsMediaSource.Factory getLiveSsMediaSourceFactory() { diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java index 7dc80a9582d..26790728e0c 100644 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java +++ b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java @@ -350,8 +350,7 @@ private static HlsMediaSource buildHlsMediaSource(final PlayerDataSource dataSou .build()); } - final NonUriHlsDataSourceFactory.Builder hlsDataSourceFactoryBuilder = - new NonUriHlsDataSourceFactory.Builder(); + final var hlsDataSourceFactoryBuilder = new NonUriHlsDataSourceFactory.Builder(); hlsDataSourceFactoryBuilder.setPlaylistString(stream.getContent()); return dataSource.getHlsMediaSourceFactory(hlsDataSourceFactoryBuilder) From c27e964d6f92b69f0f80e81923c7242ed92740a7 Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Thu, 3 Jul 2025 06:05:12 +0100 Subject: [PATCH 05/10] [SoundCloud] Refactor Soundcloud audio stream extraction code to separate building Hls and Progressive streams Add HlsAudioStream to facilitate refreshing expired Hls playlists Implement refreshing expired hls playlists in RefreshableHlsHttpDataSource --- .../RefreshableHlsHttpDataSource.java | 233 ++++++++++++++++++ .../player/helper/PlayerDataSource.java | 22 +- .../player/resolver/PlaybackResolver.java | 2 +- 3 files changed, 252 insertions(+), 5 deletions(-) create mode 100644 app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java new file mode 100644 index 00000000000..0aacf8dd6a5 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java @@ -0,0 +1,233 @@ +package org.schabi.newpipe.player.datasource; + +import android.net.Uri; +import android.util.Log; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; + +import com.google.android.exoplayer2.source.hls.playlist.HlsMediaPlaylist; +import com.google.android.exoplayer2.source.hls.playlist.HlsPlaylistParser; +import com.google.android.exoplayer2.upstream.DataSourceInputStream; +import com.google.android.exoplayer2.upstream.DataSpec; +import com.google.android.exoplayer2.upstream.HttpDataSource; + +import org.schabi.newpipe.extractor.exceptions.ExtractionException; +import org.schabi.newpipe.extractor.stream.RefreshableStream; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; + +public class RefreshableHlsHttpDataSource extends LoggingHttpDataSource { + + private final String TAG = + RefreshableHlsHttpDataSource.class.getSimpleName() + "@" + hashCode(); + private final RefreshableStream refreshableStream; + private final String originalPlaylistUrl; + private final Map chunkUrlMap = new LinkedHashMap<>(); + private int readsCalled; + private boolean isError; + + public RefreshableHlsHttpDataSource(final RefreshableStream refreshableStream) { + this.refreshableStream = refreshableStream; + originalPlaylistUrl = refreshableStream.initialUrl(); + } + + @SuppressWarnings("checkstyle:LineLength") + public RefreshableHlsHttpDataSource(final RefreshableStream refreshableStream, + @Nullable final String userAgent, + final int connectTimeoutMillis, + final int readTimeoutMillis, + final boolean allowCrossProtocolRedirects, + @Nullable final RequestProperties defaultRequestProperties) { + super(userAgent, + connectTimeoutMillis, + readTimeoutMillis, + allowCrossProtocolRedirects, + defaultRequestProperties); + this.refreshableStream = refreshableStream; + originalPlaylistUrl = refreshableStream.initialUrl(); + } + + @Override + public int read(@NonNull final byte[] buffer, final int offset, final int length) + throws HttpDataSourceException { + return super.read(buffer, offset, length); + } + + @Override + public long open(final DataSpec dataSpec) throws HttpDataSourceException { + final var url = dataSpec.uri.toString(); + Log.d(TAG, "called open(" + url + ")"); + if (!url.contains(refreshableStream.playlistId())) { + // TODO: throw error or no? + Log.e(TAG, "Playlist id does not match"); + } + return chunkUrlMap.isEmpty() + ? openInternal(dataSpec) + : openInternal(getUpdatedDataSpec(dataSpec)); + } + + private long openInternal(final DataSpec dataSpec) throws HttpDataSourceException { + try { + final var bytesToRead = super.open(dataSpec); + Log.d(TAG, "Bytes to read: " + bytesToRead); + isError = false; // if we got to this line there was no error + return bytesToRead; + } catch (final InvalidResponseCodeException e) { + // TODO: This assumes SoundCloud returning 403 when playlist expires + // If we need to refresh playlists for other services at a later date then + // need to generalize this class + if (isError || e.responseCode != 403) { + // Use isError to prevent infinite loop if playlist expires, we replace signature + // but then that one gives an error, and then we replace signature again, and so on + // The expectation is that no error will be thrown on the first recursion + throw e; + } + + try { + refreshPlaylist(); + } catch (final ExtractionException | IOException ex) { + // TODO: better error here + // TODO: so what happens when we throw exception here + // and this method gets called again in this class? + // if we want to prevent open being called again we need to throw a runtime + throw new HttpDataSourceException("Error refreshing Hls playlist: " + + originalPlaylistUrl, + new IOException(ex), dataSpec, + HttpDataSourceException.TYPE_OPEN); + } + isError = true; + // Use recursion to reuse error handling without code duplication + return openInternal(getUpdatedDataSpec(dataSpec)); + } + } + + private void refreshPlaylist() throws ExtractionException, IOException { + Log.d(TAG, "refreshPlaylist() - originalPlaylistUrl " + originalPlaylistUrl); + final var newPlaylistUrl = refreshableStream.fetchLatestUrl(); + Log.d(TAG, "New playlist url " + newPlaylistUrl); + Log.d(TAG, "Extracting new playlist Chunks"); + final var newChunks = extractChunksFromPlaylist(newPlaylistUrl); + + if (!chunkUrlMap.isEmpty()) { + updateChunkMap(chunkUrlMap, newChunks); + } + initializeChunkMappings(chunkUrlMap, newChunks); + } + + private static void initializeChunkMappings(final Map chunkMap, + final List newChunks) { + for (int i = 0; i < newChunks.size(); ++i) { + final var newUrl = newChunks.get(i); + chunkMap.put(getBaseUrl(newUrl), newUrl); + } + } + + private static void updateChunkMap(final Map chunkMap, + final List newChunks) throws IOException { + if (chunkMap.size() != newChunks.size()) { + throw new IOException("Error extracting chunks: chunks are not same size\n" + + "Expected " + chunkMap.size() + + " and got " + newChunks.size()); + } + + final var baseUrlIt = chunkMap.keySet().iterator(); + final var newChunkUrlIt = newChunks.iterator(); + while (baseUrlIt.hasNext()) { + chunkMap.put(baseUrlIt.next(), newChunkUrlIt.next()); + } + } + + private static String getBaseUrl(final String url) { + final int idx = url.indexOf('?'); + return idx == -1 ? url : url.substring(0, idx); + } + + // TODO: better name + private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { + final var currentUrl = dataSpec.uri.toString(); + Log.d(TAG, "getUpdatedDataSpec(" + currentUrl + ')'); + // Playlist has expired, so get mapping for new url + final var baseUrl = getBaseUrl(currentUrl); + + if (baseUrl.equals(currentUrl)) { + Log.e(TAG, "Url has no query parameters"); + } + + final var updatedUrl = chunkUrlMap.get(baseUrl); + if (updatedUrl == null) { + throw new IllegalStateException("baseUrl not found in mappings: " + baseUrl); + // TODO: problemo + } + Log.d(TAG, "updated url:" + updatedUrl); + return dataSpec.buildUpon() + .setUri(Uri.parse(updatedUrl)) + .build(); + } + + /** + * Extracts the chunks/segments from an m3u8 playlist using + * ExoPlayer's {@link HlsPlaylistParser}. + * @param playlistUrl url of m3u8 playlist to extract + * @return Urls for all the chunks/segments in the playlist + * @throws IOException If error extracting the chunks + */ + private List extractChunksFromPlaylist(final String playlistUrl) + throws IOException { + Log.d(TAG, "extractChunksFromPlaylist(" + playlistUrl + ')'); + final var chunks = new ArrayList(); + final var parser = new HlsPlaylistParser(); + final var dataSpec = new DataSpec(Uri.parse(playlistUrl)); + final var httpDataSource = new LoggingHttpDataSource.Factory().createDataSource(); + + // Adapted from ParsingLoadable.load() + // DataSourceInputStream opens the data source internally on open() + // It passes dataSpec to data source + // httpDataSource is a DefaultHttpDataSource, and getUri will return dataSpec's uri + // which == playlistUrl + try (@SuppressWarnings("LocalCanBeFinal") + var inputStream = new DataSourceInputStream(httpDataSource, dataSpec)) { + inputStream.open(); + + final var playlist = + parser.parse(Objects.requireNonNull(httpDataSource.getUri()), inputStream); + + if (!(playlist instanceof final HlsMediaPlaylist hlsMediaPlaylist)) { + throw new IOException("Expected Hls playlist to be an HlsMediaPlaylist, but was a " + + playlist.getClass().getSimpleName()); + } + for (final var segment : hlsMediaPlaylist.segments) { + chunks.add(segment.url); + } + Log.d(TAG, "Extracted " + chunks.size() + " chunks"); + chunks.stream().forEach(m -> Log.d(TAG, "Chunk " + m)); + return chunks; + } finally { + httpDataSource.close(); + } + } + + public static class Factory extends LoggingHttpDataSource.Factory { + private final RefreshableStream refreshableStream; + + public Factory(final RefreshableStream refreshableStream) { + this.refreshableStream = refreshableStream; + } + + @NonNull + @Override + public HttpDataSource createDataSource() { + return new RefreshableHlsHttpDataSource(refreshableStream, + userAgent, + connectTimeoutMs, + readTimeoutMs, + allowCrossProtocolRedirects, + defaultRequestProperties); + } + } +} diff --git a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java index eeee9bac563..d645069e1aa 100644 --- a/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/helper/PlayerDataSource.java @@ -26,8 +26,11 @@ import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubeOtfDashManifestCreator; import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubePostLiveStreamDvrDashManifestCreator; import org.schabi.newpipe.extractor.services.youtube.dashmanifestcreators.YoutubeProgressiveDashManifestCreator; +import org.schabi.newpipe.extractor.stream.RefreshableStream; +import org.schabi.newpipe.extractor.stream.Stream; import org.schabi.newpipe.player.datasource.LoggingHttpDataSource; import org.schabi.newpipe.player.datasource.NonUriHlsDataSourceFactory; +import org.schabi.newpipe.player.datasource.RefreshableHlsHttpDataSource; import org.schabi.newpipe.player.datasource.YoutubeHttpDataSource; import java.io.File; @@ -151,12 +154,23 @@ public DashMediaSource.Factory getLiveYoutubeDashMediaSourceFactory() { //region Generic media source factories public HlsMediaSource.Factory getHlsMediaSourceFactory( - @Nullable final NonUriHlsDataSourceFactory.Builder hlsDataSourceFactoryBuilder) { - if (hlsDataSourceFactoryBuilder != null) { - hlsDataSourceFactoryBuilder.setDataSourceFactory(cacheDataSourceFactory); - return new HlsMediaSource.Factory(hlsDataSourceFactoryBuilder.build()); + @NonNull final NonUriHlsDataSourceFactory.Builder hlsDataSourceFactoryBuilder) { + hlsDataSourceFactoryBuilder.setDataSourceFactory(cacheDataSourceFactory); + return new HlsMediaSource.Factory(hlsDataSourceFactoryBuilder.build()); + } + + public HlsMediaSource.Factory getHlsMediaSourceFactory(final Stream stream) { + if (stream instanceof final RefreshableStream refreshableStream) { + return new HlsMediaSource.Factory( + createCacheDataSourceFactory( + new RefreshableHlsHttpDataSource.Factory(refreshableStream) + ) + ); } + return getHlsMediaSourceFactory(); + } + public HlsMediaSource.Factory getHlsMediaSourceFactory() { return new HlsMediaSource.Factory(cacheDataSourceFactory); } diff --git a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java index 26790728e0c..0f84a968ee4 100644 --- a/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java +++ b/app/src/main/java/org/schabi/newpipe/player/resolver/PlaybackResolver.java @@ -342,7 +342,7 @@ private static HlsMediaSource buildHlsMediaSource(final PlayerDataSource dataSou throws ResolverException { if (stream.isUrl()) { throwResolverExceptionIfUrlNullOrEmpty(stream.getContent()); - return dataSource.getHlsMediaSourceFactory(null).createMediaSource( + return dataSource.getHlsMediaSourceFactory(stream).createMediaSource( new MediaItem.Builder() .setTag(metadata) .setUri(Uri.parse(stream.getContent())) From 53ae0467ce5cb08938ee661ab0d18628751dbc6e Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Mon, 7 Jul 2025 03:45:56 +0100 Subject: [PATCH 06/10] Add DEBUG guards for logging --- .../datasource/LoggingHttpDataSource.java | 6 +++ .../RefreshableHlsHttpDataSource.java | 52 ++++++++++++++----- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java index 3d5f1e81594..c5ffe87330a 100644 --- a/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/LoggingHttpDataSource.java @@ -1,5 +1,7 @@ package org.schabi.newpipe.player.datasource; +import static org.schabi.newpipe.MainActivity.DEBUG; + import android.util.Log; import androidx.annotation.NonNull; @@ -36,6 +38,10 @@ public LoggingHttpDataSource(@Nullable final String userAgent, @Override public long open(final DataSpec dataSpec) throws HttpDataSourceException { + if (!DEBUG) { + return super.open(dataSpec); + } + Log.d(TAG, "Request URL: " + dataSpec.uri); try { return super.open(dataSpec); diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java index 0aacf8dd6a5..a6ce450d8ca 100644 --- a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java @@ -1,5 +1,7 @@ package org.schabi.newpipe.player.datasource; +import static org.schabi.newpipe.MainActivity.DEBUG; + import android.net.Uri; import android.util.Log; @@ -62,10 +64,15 @@ public int read(@NonNull final byte[] buffer, final int offset, final int length @Override public long open(final DataSpec dataSpec) throws HttpDataSourceException { final var url = dataSpec.uri.toString(); - Log.d(TAG, "called open(" + url + ")"); + if (DEBUG) { + Log.d(TAG, "called open(" + url + ")"); + } + if (!url.contains(refreshableStream.playlistId())) { // TODO: throw error or no? - Log.e(TAG, "Playlist id does not match"); + if (DEBUG) { + Log.e(TAG, "Playlist id does not match"); + } } return chunkUrlMap.isEmpty() ? openInternal(dataSpec) @@ -75,7 +82,9 @@ public long open(final DataSpec dataSpec) throws HttpDataSourceException { private long openInternal(final DataSpec dataSpec) throws HttpDataSourceException { try { final var bytesToRead = super.open(dataSpec); - Log.d(TAG, "Bytes to read: " + bytesToRead); + if (DEBUG) { + Log.d(TAG, "Bytes to read: " + bytesToRead); + } isError = false; // if we got to this line there was no error return bytesToRead; } catch (final InvalidResponseCodeException e) { @@ -108,10 +117,16 @@ private long openInternal(final DataSpec dataSpec) throws HttpDataSourceExceptio } private void refreshPlaylist() throws ExtractionException, IOException { - Log.d(TAG, "refreshPlaylist() - originalPlaylistUrl " + originalPlaylistUrl); + if (DEBUG) { + Log.d(TAG, "refreshPlaylist() - originalPlaylistUrl " + originalPlaylistUrl); + } + final var newPlaylistUrl = refreshableStream.fetchLatestUrl(); - Log.d(TAG, "New playlist url " + newPlaylistUrl); - Log.d(TAG, "Extracting new playlist Chunks"); + + if (DEBUG) { + Log.d(TAG, "New playlist url " + newPlaylistUrl); + Log.d(TAG, "Extracting new playlist Chunks"); + } final var newChunks = extractChunksFromPlaylist(newPlaylistUrl); if (!chunkUrlMap.isEmpty()) { @@ -151,12 +166,16 @@ private static String getBaseUrl(final String url) { // TODO: better name private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { final var currentUrl = dataSpec.uri.toString(); - Log.d(TAG, "getUpdatedDataSpec(" + currentUrl + ')'); + if (DEBUG) { + Log.d(TAG, "getUpdatedDataSpec(" + currentUrl + ')'); + } // Playlist has expired, so get mapping for new url final var baseUrl = getBaseUrl(currentUrl); if (baseUrl.equals(currentUrl)) { - Log.e(TAG, "Url has no query parameters"); + if (DEBUG) { + Log.e(TAG, "Url has no query parameters"); + } } final var updatedUrl = chunkUrlMap.get(baseUrl); @@ -164,7 +183,9 @@ private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { throw new IllegalStateException("baseUrl not found in mappings: " + baseUrl); // TODO: problemo } - Log.d(TAG, "updated url:" + updatedUrl); + if (DEBUG) { + Log.d(TAG, "updated url:" + updatedUrl); + } return dataSpec.buildUpon() .setUri(Uri.parse(updatedUrl)) .build(); @@ -179,7 +200,9 @@ private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { */ private List extractChunksFromPlaylist(final String playlistUrl) throws IOException { - Log.d(TAG, "extractChunksFromPlaylist(" + playlistUrl + ')'); + if (DEBUG) { + Log.d(TAG, "extractChunksFromPlaylist(" + playlistUrl + ')'); + } final var chunks = new ArrayList(); final var parser = new HlsPlaylistParser(); final var dataSpec = new DataSpec(Uri.parse(playlistUrl)); @@ -201,11 +224,16 @@ private List extractChunksFromPlaylist(final String playlistUrl) throw new IOException("Expected Hls playlist to be an HlsMediaPlaylist, but was a " + playlist.getClass().getSimpleName()); } + for (final var segment : hlsMediaPlaylist.segments) { chunks.add(segment.url); } - Log.d(TAG, "Extracted " + chunks.size() + " chunks"); - chunks.stream().forEach(m -> Log.d(TAG, "Chunk " + m)); + + if (DEBUG) { + Log.d(TAG, "Extracted " + chunks.size() + " chunks"); + chunks.stream().forEach(m -> Log.d(TAG, "Chunk " + m)); + } + return chunks; } finally { httpDataSource.close(); From d302c3cc9c55a2cbb8498b8e99a3af426c41d331 Mon Sep 17 00:00:00 2001 From: David Asunmo <22662897+davidasunmo@users.noreply.github.com.> Date: Tue, 15 Jul 2025 10:24:44 +0100 Subject: [PATCH 07/10] Rename baseUrl to removeQueryParameters Remove read override as not needed anymore Remove some TODOs --- .../org/schabi/newpipe/player/Player.java | 38 +++++++++---------- .../RefreshableHlsHttpDataSource.java | 21 ++-------- .../player/playback/MediaSourceManager.java | 5 --- .../org/schabi/newpipe/util/InfoCache.java | 4 -- .../org/schabi/newpipe/util/ListHelper.java | 2 +- 5 files changed, 24 insertions(+), 46 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/Player.java b/app/src/main/java/org/schabi/newpipe/player/Player.java index be3cea5ff00..648e3511f68 100644 --- a/app/src/main/java/org/schabi/newpipe/player/Player.java +++ b/app/src/main/java/org/schabi/newpipe/player/Player.java @@ -1055,24 +1055,6 @@ private Disposable getProgressUpdateDisposable() { //endregion - - /*////////////////////////////////////////////////////////////////////////// - // Playback states - //////////////////////////////////////////////////////////////////////////*/ - //region Playback states - @Override - public void onPlayWhenReadyChanged(final boolean playWhenReady, final int reason) { - if (DEBUG) { - Log.d(TAG, "ExoPlayer - onPlayWhenReadyChanged() called with: " - + "playWhenReady = [" + playWhenReady + "], " - + "reason = [" + reason + "]"); - } - final int playbackState = exoPlayerIsNull() - ? com.google.android.exoplayer2.Player.STATE_IDLE - : simpleExoPlayer.getPlaybackState(); - updatePlaybackState(playWhenReady, playbackState); - } - public static String exoplayerStateToString(final int playbackState) { return switch (playbackState) { case com.google.android.exoplayer2.Player.STATE_IDLE -> // 1 @@ -1101,6 +1083,24 @@ public static String stateToString(final int state) { }; } + + /*////////////////////////////////////////////////////////////////////////// + // Playback states + //////////////////////////////////////////////////////////////////////////*/ + //region Playback states + @Override + public void onPlayWhenReadyChanged(final boolean playWhenReady, final int reason) { + if (DEBUG) { + Log.d(TAG, "ExoPlayer - onPlayWhenReadyChanged() called with: " + + "playWhenReady = [" + playWhenReady + "], " + + "reason = [" + reason + "]"); + } + final int playbackState = exoPlayerIsNull() + ? com.google.android.exoplayer2.Player.STATE_IDLE + : simpleExoPlayer.getPlaybackState(); + updatePlaybackState(playWhenReady, playbackState); + } + @Override public void onPlaybackStateChanged(final int playbackState) { if (DEBUG) { @@ -1114,7 +1114,7 @@ private void updatePlaybackState(final boolean playWhenReady, final int playback if (DEBUG) { Log.d(TAG, "ExoPlayer - updatePlaybackState() called with: " + "playWhenReady = [" + playWhenReady + "], " - + "playbackState = [" + playbackState + "]"); + + "playbackState = [" + exoplayerStateToString(playbackState) + "]"); } if (currentState == STATE_PAUSED_SEEK) { diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java index a6ce450d8ca..929c577c0f4 100644 --- a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java @@ -31,7 +31,6 @@ public class RefreshableHlsHttpDataSource extends LoggingHttpDataSource { private final RefreshableStream refreshableStream; private final String originalPlaylistUrl; private final Map chunkUrlMap = new LinkedHashMap<>(); - private int readsCalled; private boolean isError; public RefreshableHlsHttpDataSource(final RefreshableStream refreshableStream) { @@ -55,12 +54,6 @@ public RefreshableHlsHttpDataSource(final RefreshableStream refreshableStream, originalPlaylistUrl = refreshableStream.initialUrl(); } - @Override - public int read(@NonNull final byte[] buffer, final int offset, final int length) - throws HttpDataSourceException { - return super.read(buffer, offset, length); - } - @Override public long open(final DataSpec dataSpec) throws HttpDataSourceException { final var url = dataSpec.uri.toString(); @@ -101,10 +94,6 @@ private long openInternal(final DataSpec dataSpec) throws HttpDataSourceExceptio try { refreshPlaylist(); } catch (final ExtractionException | IOException ex) { - // TODO: better error here - // TODO: so what happens when we throw exception here - // and this method gets called again in this class? - // if we want to prevent open being called again we need to throw a runtime throw new HttpDataSourceException("Error refreshing Hls playlist: " + originalPlaylistUrl, new IOException(ex), dataSpec, @@ -139,7 +128,7 @@ private static void initializeChunkMappings(final Map chunkMap, final List newChunks) { for (int i = 0; i < newChunks.size(); ++i) { final var newUrl = newChunks.get(i); - chunkMap.put(getBaseUrl(newUrl), newUrl); + chunkMap.put(removeQueryParameters(newUrl), newUrl); } } @@ -158,19 +147,18 @@ private static void updateChunkMap(final Map chunkMap, } } - private static String getBaseUrl(final String url) { + private static String removeQueryParameters(final String url) { final int idx = url.indexOf('?'); return idx == -1 ? url : url.substring(0, idx); } - // TODO: better name private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { final var currentUrl = dataSpec.uri.toString(); if (DEBUG) { Log.d(TAG, "getUpdatedDataSpec(" + currentUrl + ')'); } - // Playlist has expired, so get mapping for new url - final var baseUrl = getBaseUrl(currentUrl); + + final var baseUrl = removeQueryParameters(currentUrl); if (baseUrl.equals(currentUrl)) { if (DEBUG) { @@ -181,7 +169,6 @@ private DataSpec getUpdatedDataSpec(final DataSpec dataSpec) { final var updatedUrl = chunkUrlMap.get(baseUrl); if (updatedUrl == null) { throw new IllegalStateException("baseUrl not found in mappings: " + baseUrl); - // TODO: problemo } if (DEBUG) { Log.d(TAG, "updated url:" + updatedUrl); diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java index dde4cd0a6ef..fb641ffd82d 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java @@ -447,11 +447,6 @@ private Single getLoadedMediaSource(@NonNull final PlayQueue MediaItemTag.from(source.getMediaItem()) .map(tag -> { final int serviceId = streamInfo.getServiceId(); - // CHECKSTYLE:OFF - // TODO: So this expiration is false. Expiration starts from when stream is extracted - // But here it is giving it fresh expiration as though the stream info is new, but it's not - // So cache expiration is not 1-1 with stream expiration, but is it supposed to be? - // CHECKSTYLE:ON final long expiration = System.currentTimeMillis() + getCacheExpirationMillis(serviceId); return new LoadedMediaSource(source, tag, stream, diff --git a/app/src/main/java/org/schabi/newpipe/util/InfoCache.java b/app/src/main/java/org/schabi/newpipe/util/InfoCache.java index 7172f4a18ef..e8a91f6ea55 100644 --- a/app/src/main/java/org/schabi/newpipe/util/InfoCache.java +++ b/app/src/main/java/org/schabi/newpipe/util/InfoCache.java @@ -90,7 +90,6 @@ private static Info getInfo(@NonNull final String key) { } if (data.isExpired()) { - // TODO: oh so we do check expiry on retrieval! LRU_CACHE.remove(key); return null; } @@ -117,9 +116,6 @@ public void putInfo(final int serviceId, @NonNull final Info info, @NonNull final Type cacheType) { final long expirationMillis = ServiceHelper.getCacheExpirationMillis(info.getServiceId()); - // CHECKSTYLE:OFF - // TODO: Expired items get refreshed by being put again; they are not removed when they expire - // CHECKSTYLE:ON if (DEBUG) { final var expiryDateInstant = Instant.now().plusMillis(expirationMillis); final var expiryDate = LocalDateTime.ofInstant(expiryDateInstant, diff --git a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java index 6b921a9daa6..dff73475c9b 100644 --- a/app/src/main/java/org/schabi/newpipe/util/ListHelper.java +++ b/app/src/main/java/org/schabi/newpipe/util/ListHelper.java @@ -304,7 +304,7 @@ public static List getFilteredAudioStreams( for (final AudioStream stream : audioStreams) { // TODO: this doesn't add HLS OPUS streams, but soundcloud has that. // Meaning it never actually plays the OPUS soundcloud streams, only - // progressive and hls mp3 + // progressive and hls mp3. So should we change this to allow HLS OPUS? if (stream.getDeliveryMethod() == DeliveryMethod.TORRENT || (stream.getDeliveryMethod() == DeliveryMethod.HLS && stream.getFormat() == MediaFormat.OPUS)) { From 89b6a74709633d2cf9f23b3ddc7c9fbbcf5f475f Mon Sep 17 00:00:00 2001 From: AbsurdlyLongUsername <22662897+absurdlylongusername@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:10:41 +0000 Subject: [PATCH 08/10] Code review: add warn and debug log overloads with Throwable --- .../main/java/org/schabi/newpipe/MainActivity.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/app/src/main/java/org/schabi/newpipe/MainActivity.java b/app/src/main/java/org/schabi/newpipe/MainActivity.java index c6c5e9b8e24..e374de3dd46 100644 --- a/app/src/main/java/org/schabi/newpipe/MainActivity.java +++ b/app/src/main/java/org/schabi/newpipe/MainActivity.java @@ -146,11 +146,25 @@ public void debug(final String tag, final String message) { Log.d(tag, message); } + @Override + public void debug(final String tag, + final String message, + final Throwable throwable) { + Log.d(tag, message, throwable); + } + @Override public void warn(final String tag, final String message) { Log.w(tag, message); } + @Override + public void warn(final String tag, + final String message, + final Throwable throwable) { + Log.w(tag, message, throwable); + } + @Override public void error(final String tag, final String message) { Log.e(tag, message); From c33bcd83c23cacc96fc528e6bfeedb68c46d544e Mon Sep 17 00:00:00 2001 From: AbsurdlyLongUsername <22662897+absurdlylongusername@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:11:52 +0000 Subject: [PATCH 09/10] Code review: Remove TODO, Add error message for missing m3u8 playlist id in soundcloud stream content chunk url --- .../datasource/RefreshableHlsHttpDataSource.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java index 929c577c0f4..b11cd7aef95 100644 --- a/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java +++ b/app/src/main/java/org/schabi/newpipe/player/datasource/RefreshableHlsHttpDataSource.java @@ -62,10 +62,18 @@ public long open(final DataSpec dataSpec) throws HttpDataSourceException { } if (!url.contains(refreshableStream.playlistId())) { - // TODO: throw error or no? if (DEBUG) { Log.e(TAG, "Playlist id does not match"); } + + final var errorMsg = String.format(""" + HLS m3u8 playlist does not contain expected playlist id. + Expected: %s + Actual url: %s""", + refreshableStream.playlistId(), + url); + + throw new IllegalStateException(errorMsg); } return chunkUrlMap.isEmpty() ? openInternal(dataSpec) From ffea07c36c4d93fa2edf94c7cb26c5769c46e718 Mon Sep 17 00:00:00 2001 From: AbsurdlyLongUsername <22662897+absurdlylongusername@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:12:45 +0000 Subject: [PATCH 10/10] Update extractor jitpack commit reference --- gradle/libs.versions.toml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 5448848a2c5..33256ab0ccd 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -49,7 +49,7 @@ sonarqube = "7.2.2.6593" statesaver = "1.4.1" # TODO: Drop because it is deprecated and incompatible with KSP2 stetho = "1.6.0" swiperefreshlayout = "1.2.0" -# You can use a local version by uncommenting a few lines in settings.gradle +# You can use a local version by uncommenting a few lines in settings.gradle.kts # Or you can use a commit you pushed to GitHub by just replacing TeamNewPipe with your GitHub # name and the commit hash with the commit hash of the (pushed) commit you want to test # This works thanks to JitPack: https://jitpack.io/ @@ -59,7 +59,8 @@ teamnewpipe-nanojson = "e9d656ddb49a412a5a0a5d5ef20ca7ef09549996" # the corresponding commit hash, since JitPack sometimes deletes artifacts. # If there’s already a git hash, just add more of it to the end (or remove a letter) # to cause jitpack to regenerate the artifact. -teamnewpipe-newpipe-extractor = "v0.25.1" +teamnewpipe-newpipe-extractor = "5b17e3deb4" +#teamnewpipe-newpipe-extractor = "v0.25.1" viewpager2 = "1.1.0" webkit = "1.14.0" # Newer versions require minSdk >= 23 work = "2.10.5" # Newer versions require minSdk >= 23 @@ -113,7 +114,7 @@ lisawray-groupie-core = { module = "com.github.lisawray.groupie:groupie", versio lisawray-groupie-viewbinding = { module = "com.github.lisawray.groupie:groupie-viewbinding", version.ref = "groupie" } livefront-bridge = { module = "com.github.livefront:bridge", version.ref = "bridge" } mockito-core = { module = "org.mockito:mockito-core", version.ref = "mockitoCore" } -newpipe-extractor = { module = "com.github.TeamNewPipe:NewPipeExtractor", version.ref = "teamnewpipe-newpipe-extractor" } +newpipe-extractor = { module = "com.github.absurdlylongusername:NewPipeExtractor", version.ref = "teamnewpipe-newpipe-extractor" } newpipe-filepicker = { module = "com.github.TeamNewPipe:NoNonsense-FilePicker", version.ref = "teamnewpipe-filepicker" } newpipe-nanojson = { module = "com.github.TeamNewPipe:nanojson", version.ref = "teamnewpipe-nanojson" } noties-markwon-core = { module = "io.noties.markwon:core", version.ref = "markwon" }