From 4ce721e9594bcaf19f243a008d71ee72dd5d1acf Mon Sep 17 00:00:00 2001 From: Mira Date: Fri, 2 Jan 2026 18:31:31 +0100 Subject: [PATCH] Apply improvements from code review This simplifies the code by not serializing and de-serializing the tags data, and also removes the DownloadManagerService.EXTRA_SOURCE field, which is always inferred from the StreamInfo. --- .../newpipe/streams/OggFromWebMWriter.java | 56 +++++++++---------- .../giga/service/DownloadManagerService.java | 5 +- 2 files changed, 26 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/streams/OggFromWebMWriter.java b/app/src/main/java/org/schabi/newpipe/streams/OggFromWebMWriter.java index b3d095e8e..ac1ed95ad 100644 --- a/app/src/main/java/org/schabi/newpipe/streams/OggFromWebMWriter.java +++ b/app/src/main/java/org/schabi/newpipe/streams/OggFromWebMWriter.java @@ -1,11 +1,11 @@ package org.schabi.newpipe.streams; import android.util.Log; +import android.util.Pair; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import org.schabi.newpipe.BuildConfig; import org.schabi.newpipe.extractor.stream.StreamInfo; import org.schabi.newpipe.streams.WebMReader.Cluster; import org.schabi.newpipe.streams.WebMReader.Segment; @@ -17,8 +17,9 @@ import java.io.Closeable; import java.io.IOException; import java.nio.ByteBuffer; import java.nio.ByteOrder; -import java.time.OffsetDateTime; import java.time.format.DateTimeFormatter; +import java.util.ArrayList; +import java.util.List; import java.util.stream.Collectors; /** @@ -284,24 +285,22 @@ public class OggFromWebMWriter implements Closeable { Log.d("OggFromWebMWriter", "Downloading media with codec ID " + webmTrack.codecId); if ("A_OPUS".equals(webmTrack.codecId)) { - var metadata = ""; - metadata += String.format("COMMENT=Downloaded using NewPipe %s on %s\n", - BuildConfig.VERSION_NAME, - OffsetDateTime.now().toString()); + final var metadata = new ArrayList>(); if (streamInfo != null) { - metadata += String.format("COMMENT=URL: %s\n", streamInfo.getUrl()); - metadata += String.format("GENRE=%s\n", streamInfo.getCategory()); - metadata += String.format("ARTIST=%s\n", streamInfo.getUploaderName()); - metadata += String.format("TITLE=%s\n", streamInfo.getName()); - metadata += String.format("DATE=%s\n", - streamInfo - .getUploadDate() - .getLocalDateTime() - .format(DateTimeFormatter.ISO_DATE)); + metadata.add(Pair.create("COMMENT", streamInfo.getUrl())); + metadata.add(Pair.create("GENRE", streamInfo.getCategory())); + metadata.add(Pair.create("ARTIST", streamInfo.getUploaderName())); + metadata.add(Pair.create("TITLE", streamInfo.getName())); + metadata.add(Pair.create("DATE", streamInfo + .getUploadDate() + .getLocalDateTime() + .format(DateTimeFormatter.ISO_DATE))); } Log.d("OggFromWebMWriter", "Creating metadata header with this data:"); - Log.d("OggFromWebMWriter", metadata); + metadata.forEach(p -> { + Log.d("OggFromWebMWriter", p.first + "=" + p.second); + }); return makeOpusTagsHeader(metadata); } else if ("A_VORBIS".equals(webmTrack.codecId)) { @@ -322,17 +321,13 @@ public class OggFromWebMWriter implements Closeable { * byte string length field and includes the string as-is. This cannot be used independently, * but must follow a proper "OpusTags" header. * - * @param keyValue A key-value pair in the format "KEY=some value" + * @param pair A key-value pair in the format "KEY=some value" * @return The binary data of the encoded metadata tag */ - private static byte[] makeOpusMetadataTag(final String keyValue) { - // Ensure the key is uppercase - final var delimiterIndex = keyValue.indexOf('='); - final var key = keyValue.substring(0, delimiterIndex).toUpperCase(); - final var value = keyValue.substring(delimiterIndex + 1); - final var reconstructedKeyValue = key + "=" + value; + private static byte[] makeOpusMetadataTag(final Pair pair) { + final var keyValue = pair.first.toUpperCase() + "=" + pair.second.trim(); - final var bytes = reconstructedKeyValue.getBytes(); + final var bytes = keyValue.getBytes(); final var buf = ByteBuffer.allocate(4 + bytes.length); buf.order(ByteOrder.LITTLE_ENDIAN); buf.putInt(bytes.length); @@ -341,20 +336,19 @@ public class OggFromWebMWriter implements Closeable { } /** - * This returns a complete "OpusTags" header, created from the provided tags string. + * This returns a complete "OpusTags" header, created from the provided metadata tags. *

* You probably want to use makeOpusMetadata(), which uses this function to create * a header with sensible metadata filled in. * - * @param keyValueLines A multiline string with each line containing a key-value pair - * in the format "KEY=some value". This may also be a blank string. + * @param keyValueLines A list of pairs of the tags. This can also be though of as a mapping + * from one key to multiple values. * @return The binary header */ - private static byte[] makeOpusTagsHeader(@NonNull final String keyValueLines) { + private static byte[] makeOpusTagsHeader(final List> keyValueLines) { final var tags = keyValueLines - .lines() - .map(String::trim) - .filter(s -> !s.isBlank()) + .stream() + .filter(p -> !p.second.isBlank()) .map(OggFromWebMWriter::makeOpusMetadataTag) .collect(Collectors.toUnmodifiableList()); diff --git a/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java b/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java index 40df6e0f5..76da18b2d 100755 --- a/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java +++ b/app/src/main/java/us/shandian/giga/service/DownloadManagerService.java @@ -75,7 +75,6 @@ public class DownloadManagerService extends Service { private static final String EXTRA_THREADS = "DownloadManagerService.extra.threads"; private static final String EXTRA_POSTPROCESSING_NAME = "DownloadManagerService.extra.postprocessingName"; private static final String EXTRA_POSTPROCESSING_ARGS = "DownloadManagerService.extra.postprocessingArgs"; - private static final String EXTRA_SOURCE = "DownloadManagerService.extra.source"; private static final String EXTRA_NEAR_LENGTH = "DownloadManagerService.extra.nearLength"; private static final String EXTRA_PATH = "DownloadManagerService.extra.storagePath"; private static final String EXTRA_PARENT_PATH = "DownloadManagerService.extra.storageParentPath"; @@ -369,7 +368,6 @@ public class DownloadManagerService extends Service { .putExtra(EXTRA_URLS, urls) .putExtra(EXTRA_KIND, kind) .putExtra(EXTRA_THREADS, threads) - .putExtra(EXTRA_SOURCE, streamInfo.getUrl()) .putExtra(EXTRA_POSTPROCESSING_NAME, psName) .putExtra(EXTRA_POSTPROCESSING_ARGS, psArgs) .putExtra(EXTRA_NEAR_LENGTH, nearLength) @@ -390,7 +388,6 @@ public class DownloadManagerService extends Service { char kind = intent.getCharExtra(EXTRA_KIND, '?'); String psName = intent.getStringExtra(EXTRA_POSTPROCESSING_NAME); String[] psArgs = intent.getStringArrayExtra(EXTRA_POSTPROCESSING_ARGS); - String source = intent.getStringExtra(EXTRA_SOURCE); long nearLength = intent.getLongExtra(EXTRA_NEAR_LENGTH, 0); String tag = intent.getStringExtra(EXTRA_STORAGE_TAG); StreamInfo streamInfo = (StreamInfo)intent.getSerializableExtra(EXTRA_STREAM_INFO); @@ -413,7 +410,7 @@ public class DownloadManagerService extends Service { final DownloadMission mission = new DownloadMission(urls, storage, kind, ps); mission.threadCount = threads; - mission.source = source; + mission.source = streamInfo.getUrl(); mission.nearLength = nearLength; mission.recoveryInfo = recovery.toArray(new MissionRecoveryInfo[0]);