From 044639c32bb715882685570be08ee3d6662bb075 Mon Sep 17 00:00:00 2001 From: Stypox Date: Wed, 18 May 2022 12:39:41 +0200 Subject: [PATCH] Solve some review comments --- .../BandcampRadioStreamExtractor.java | 9 +- .../services/youtube/DeliveryType.java | 2 +- .../YoutubeDashManifestCreatorsUtils.java | 69 ++---------- .../extractors/YoutubeStreamExtractor.java | 102 ++++++++---------- .../newpipe/extractor/stream/StreamInfo.java | 2 +- 5 files changed, 63 insertions(+), 121 deletions(-) diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/bandcamp/extractors/BandcampRadioStreamExtractor.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/bandcamp/extractors/BandcampRadioStreamExtractor.java index 9668b803..5724d371 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/bandcamp/extractors/BandcampRadioStreamExtractor.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/bandcamp/extractors/BandcampRadioStreamExtractor.java @@ -5,6 +5,7 @@ import com.grack.nanojson.JsonObject; import com.grack.nanojson.JsonParser; import com.grack.nanojson.JsonParserException; import org.jsoup.Jsoup; +import org.jsoup.nodes.Element; import org.schabi.newpipe.extractor.MediaFormat; import org.schabi.newpipe.extractor.NewPipe; import org.schabi.newpipe.extractor.StreamingService; @@ -80,9 +81,11 @@ public class BandcampRadioStreamExtractor extends BandcampStreamExtractor { @Nonnull @Override - public String getUploaderName() { - return Jsoup.parse(showInfo.getString("image_caption")) - .getElementsByTag("a").first().text(); + public String getUploaderName() throws ParsingException { + return Jsoup.parse(showInfo.getString("image_caption")).getElementsByTag("a").stream() + .map(Element::text) + .findFirst() + .orElseThrow(() -> new ParsingException("Could not get uploader name")); } @Nullable diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/DeliveryType.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/DeliveryType.java index 0e5b3450..17833dc5 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/DeliveryType.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/DeliveryType.java @@ -4,7 +4,7 @@ package org.schabi.newpipe.extractor.services.youtube; * Streaming format types used by YouTube in their streams. * *

- * It is different of {@link org.schabi.newpipe.extractor.stream.DeliveryMethod delivery methods}! + * It is different from {@link org.schabi.newpipe.extractor.stream.DeliveryMethod delivery methods}! *

*/ public enum DeliveryType { diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/dashmanifestcreators/YoutubeDashManifestCreatorsUtils.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/dashmanifestcreators/YoutubeDashManifestCreatorsUtils.java index 7f3c5232..ec876c67 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/dashmanifestcreators/YoutubeDashManifestCreatorsUtils.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/dashmanifestcreators/YoutubeDashManifestCreatorsUtils.java @@ -83,59 +83,18 @@ public final class YoutubeDashManifestCreatorsUtils { */ public static final String ALR_YES = "&alr=yes"; - /** - * Constant which represents the {@code MPD} element of DASH manifests. - */ + // XML elements of DASH MPD manifests + // see https://www.brendanlong.com/the-structure-of-an-mpeg-dash-mpd.html public static final String MPD = "MPD"; - - /** - * Constant which represents the {@code Period} element of DASH manifests. - */ public static final String PERIOD = "Period"; - - /** - * Constant which represents the {@code AdaptationSet} element of DASH manifests. - */ public static final String ADAPTATION_SET = "AdaptationSet"; - - /** - * Constant which represents the {@code Role} element of DASH manifests. - */ public static final String ROLE = "Role"; - - /** - * Constant which represents the {@code Representation} element of DASH manifests. - */ public static final String REPRESENTATION = "Representation"; - - /** - * Constant which represents the {@code AudioChannelConfiguration} element of DASH manifests. - */ public static final String AUDIO_CHANNEL_CONFIGURATION = "AudioChannelConfiguration"; - - /** - * Constant which represents the {@code SegmentTemplate} element of DASH manifests. - */ public static final String SEGMENT_TEMPLATE = "SegmentTemplate"; - - /** - * Constant which represents the {@code SegmentTimeline} element of DASH manifests. - */ public static final String SEGMENT_TIMELINE = "SegmentTimeline"; - - /** - * Constant which represents the {@code SegmentBase} element of DASH manifests. - */ public static final String BASE_URL = "BaseURL"; - - /** - * Constant which represents the {@code SegmentBase} element of DASH manifests. - */ public static final String SEGMENT_BASE = "SegmentBase"; - - /** - * Constant which represents the {@code Initialization} element of DASH manifests. - */ public static final String INITIALIZATION = "Initialization"; /** @@ -665,7 +624,7 @@ public final class YoutubeDashManifestCreatorsUtils { if (isHtml5StreamingUrl) { baseStreamingUrl += ALR_YES; } - baseStreamingUrl = appendRnParamAndSqParamIfNeeded(baseStreamingUrl, deliveryType); + baseStreamingUrl = appendRnSqParamsIfNeeded(baseStreamingUrl, deliveryType); final Downloader downloader = NewPipe.getDownloader(); if (isHtml5StreamingUrl) { @@ -701,7 +660,7 @@ public final class YoutubeDashManifestCreatorsUtils { * {@link XMLConstants#ACCESS_EXTERNAL_SCHEMA} in {@link DocumentBuilderFactory} instances. * * @return an instance of {@link Document} secured against XXE attacks on supported platforms, - * that should then be convertible to an XML string without security problems + * that should then be convertible to an XML string without security problems */ private static Document newDocument() throws ParserConfigurationException { final DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance(); @@ -709,8 +668,8 @@ public final class YoutubeDashManifestCreatorsUtils { documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); documentBuilderFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); } catch (final Exception ignored) { - // Ignore exceptions as setting these attributes to secure XML generation is supported - // by all platforms (like the Android implementation) + // Ignore exceptions as setting these attributes to secure XML generation is not + // supported by all platforms (like the Android implementation) } final DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder(); @@ -736,8 +695,8 @@ public final class YoutubeDashManifestCreatorsUtils { transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); } catch (final Exception ignored) { - // Ignore exceptions as setting these attributes to secure XML generation is supported - // by all platforms (like the Android implementation) + // Ignore exceptions as setting these attributes to secure XML generation is not + // supported by all platforms (like the Android implementation) } final Transformer transformer = transformerFactory.newTransformer(); @@ -759,16 +718,10 @@ public final class YoutubeDashManifestCreatorsUtils { * @return the base streaming URL to which the param(s) are appended, depending on the * {@link DeliveryType} of the stream */ - @SuppressWarnings({"checkstyle:FinalParameters", "checkstyle:FinalLocalVariable"}) @Nonnull - private static String appendRnParamAndSqParamIfNeeded( - @Nonnull String baseStreamingUrl, - @Nonnull final DeliveryType deliveryType) { - if (deliveryType != DeliveryType.PROGRESSIVE) { - baseStreamingUrl += SQ_0; - } - - return baseStreamingUrl + RN_0; + private static String appendRnSqParamsIfNeeded(@Nonnull final String baseStreamingUrl, + @Nonnull final DeliveryType deliveryType) { + return baseStreamingUrl + (deliveryType == DeliveryType.PROGRESSIVE ? "" : SQ_0) + RN_0; } /** diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java index 01270012..5e57b2a1 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/services/youtube/extractors/YoutubeStreamExtractor.java @@ -1140,17 +1140,11 @@ public class YoutubeStreamExtractor extends StreamExtractor { return videoSecondaryInfoRenderer; } - @FunctionalInterface - private interface StreamBuilderHelper { - @Nonnull - T buildStream(ItagInfo itagInfo); - } - @Nonnull private List getItags( final String streamingDataKey, final ItagItem.ItagType itagTypeWanted, - final StreamBuilderHelper streamBuilderHelper, + final java.util.function.Function streamBuilderHelper, final String streamTypeExceptionMessage) throws ParsingException { try { final List itagInfos = new ArrayList<>(); @@ -1176,7 +1170,7 @@ public class YoutubeStreamExtractor extends StreamExtractor { final List streamList = new ArrayList<>(); for (final ItagInfo itagInfo : itagInfos) { - final T stream = streamBuilderHelper.buildStream(itagInfo); + final T stream = streamBuilderHelper.apply(itagInfo); if (!Stream.containSimilarStream(stream, streamList)) { streamList.add(stream); } @@ -1190,8 +1184,8 @@ public class YoutubeStreamExtractor extends StreamExtractor { } /** - * Get the {@link StreamBuilderHelper} which will be used to build {@link AudioStream}s in - * {@link #getItags(String, ItagItem.ItagType, StreamBuilderHelper, String)} + * Get the stream builder helper which will be used to build {@link AudioStream}s in + * {@link #getItags(String, ItagItem.ItagType, java.util.function.Function, String)} * *

* The {@code StreamBuilderHelper} will set the following attributes in the @@ -1213,38 +1207,34 @@ public class YoutubeStreamExtractor extends StreamExtractor { * Note that the {@link ItagItem} comes from an {@link ItagInfo} instance. *

* - * @return a {@link StreamBuilderHelper} to build {@link AudioStream}s + * @return a stream builder helper to build {@link AudioStream}s */ @Nonnull - private StreamBuilderHelper getAudioStreamBuilderHelper() { - return new StreamBuilderHelper() { - @Nonnull - @Override - public AudioStream buildStream(@Nonnull final ItagInfo itagInfo) { - final ItagItem itagItem = itagInfo.getItagItem(); - final AudioStream.Builder builder = new AudioStream.Builder() - .setId(String.valueOf(itagItem.id)) - .setContent(itagInfo.getContent(), itagInfo.getIsUrl()) - .setMediaFormat(itagItem.getMediaFormat()) - .setAverageBitrate(itagItem.getAverageBitrate()) - .setItagItem(itagItem); + private java.util.function.Function getAudioStreamBuilderHelper() { + return (itagInfo) -> { + final ItagItem itagItem = itagInfo.getItagItem(); + final AudioStream.Builder builder = new AudioStream.Builder() + .setId(String.valueOf(itagItem.id)) + .setContent(itagInfo.getContent(), itagInfo.getIsUrl()) + .setMediaFormat(itagItem.getMediaFormat()) + .setAverageBitrate(itagItem.getAverageBitrate()) + .setItagItem(itagItem); - if (streamType == StreamType.LIVE_STREAM - || streamType == StreamType.POST_LIVE_STREAM - || !itagInfo.getIsUrl()) { - // For YouTube videos on OTF streams and for all streams of post-live streams - // and live streams, only the DASH delivery method can be used. - builder.setDeliveryMethod(DeliveryMethod.DASH); - } - - return builder.build(); + if (streamType == StreamType.LIVE_STREAM + || streamType == StreamType.POST_LIVE_STREAM + || !itagInfo.getIsUrl()) { + // For YouTube videos on OTF streams and for all streams of post-live streams + // and live streams, only the DASH delivery method can be used. + builder.setDeliveryMethod(DeliveryMethod.DASH); } + + return builder.build(); }; } /** - * Get the {@link StreamBuilderHelper} which will be used to build {@link VideoStream}s in - * {@link #getItags(String, ItagItem.ItagType, StreamBuilderHelper, String)} + * Get the stream builder helper which will be used to build {@link VideoStream}s in + * {@link #getItags(String, ItagItem.ItagType, java.util.function.Function, String)} * *

* The {@code StreamBuilderHelper} will set the following attributes in the @@ -1272,37 +1262,33 @@ public class YoutubeStreamExtractor extends StreamExtractor { * Note that the {@link ItagItem} comes from an {@link ItagInfo} instance. *

* - * @param areStreamsVideoOnly whether the {@link StreamBuilderHelper} will set the video + * @param areStreamsVideoOnly whether the stream builder helper will set the video * streams as video-only streams - * @return a {@link StreamBuilderHelper} to build {@link VideoStream}s + * @return a stream builder helper to build {@link VideoStream}s */ @Nonnull - private StreamBuilderHelper getVideoStreamBuilderHelper( + private java.util.function.Function getVideoStreamBuilderHelper( final boolean areStreamsVideoOnly) { - return new StreamBuilderHelper() { - @Nonnull - @Override - public VideoStream buildStream(@Nonnull final ItagInfo itagInfo) { - final ItagItem itagItem = itagInfo.getItagItem(); - final VideoStream.Builder builder = new VideoStream.Builder() - .setId(String.valueOf(itagItem.id)) - .setContent(itagInfo.getContent(), itagInfo.getIsUrl()) - .setMediaFormat(itagItem.getMediaFormat()) - .setIsVideoOnly(areStreamsVideoOnly) - .setItagItem(itagItem); + return (itagInfo) -> { + final ItagItem itagItem = itagInfo.getItagItem(); + final VideoStream.Builder builder = new VideoStream.Builder() + .setId(String.valueOf(itagItem.id)) + .setContent(itagInfo.getContent(), itagInfo.getIsUrl()) + .setMediaFormat(itagItem.getMediaFormat()) + .setIsVideoOnly(areStreamsVideoOnly) + .setItagItem(itagItem); - final String resolutionString = itagItem.getResolutionString(); - builder.setResolution(resolutionString != null ? resolutionString - : EMPTY_STRING); + final String resolutionString = itagItem.getResolutionString(); + builder.setResolution(resolutionString != null ? resolutionString + : EMPTY_STRING); - if (streamType != StreamType.VIDEO_STREAM || !itagInfo.getIsUrl()) { - // For YouTube videos on OTF streams and for all streams of post-live streams - // and live streams, only the DASH delivery method can be used. - builder.setDeliveryMethod(DeliveryMethod.DASH); - } - - return builder.build(); + if (streamType != StreamType.VIDEO_STREAM || !itagInfo.getIsUrl()) { + // For YouTube videos on OTF streams and for all streams of post-live streams + // and live streams, only the DASH delivery method can be used. + builder.setDeliveryMethod(DeliveryMethod.DASH); } + + return builder.build(); }; } diff --git a/extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java b/extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java index c067221c..8d3f2c52 100644 --- a/extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java +++ b/extractor/src/main/java/org/schabi/newpipe/extractor/stream/StreamInfo.java @@ -114,7 +114,7 @@ public class StreamInfo extends Info { final String name = extractor.getName(); final int ageLimit = extractor.getAgeLimit(); - // Suppress always-non-null warning as here we double-check it is really not null + // Suppress always-non-null warning as here we double-check it really is not null //noinspection ConstantConditions if (streamType == StreamType.NONE || isNullOrEmpty(url)