Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix downloads of streams with missing MediaFormat #10165

Merged
merged 5 commits into from
Sep 19, 2023
Merged

Fix downloads of streams with missing MediaFormat #10165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0db12e5
Rename StreamSizeWrapper to StreamInfoWrapper
TobiGr Jun 15, 2023
f3859ed
Retrieve MediaFormat for streams that could not be extracted by the e…
TobiGr Aug 14, 2023
e510671
Add tests for new methods retrieving MediaFormats
TobiGr Aug 14, 2023
ba84e7e
Display "Unknown quality" if quality is unknown and not MediaFormat name
TobiGr Jul 19, 2023
992bb5d
Simplify retrieveMediaFormatFromContentTypeHeader
Stypox Sep 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 165 additions & 4 deletions app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,21 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.MediumTest
import androidx.test.internal.runner.junit4.statement.UiThreadStatement
import org.junit.Assert
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.schabi.newpipe.R
import org.schabi.newpipe.extractor.MediaFormat
import org.schabi.newpipe.extractor.downloader.Response
import org.schabi.newpipe.extractor.stream.AudioStream
import org.schabi.newpipe.extractor.stream.Stream
import org.schabi.newpipe.extractor.stream.SubtitlesStream
import org.schabi.newpipe.extractor.stream.VideoStream
import org.schabi.newpipe.util.StreamItemAdapter.StreamInfoWrapper

@MediumTest
@RunWith(AndroidJUnit4::class)
Expand Down Expand Up @@ -84,7 +90,7 @@ class StreamItemAdapterTest {
@Test
fun subtitleStreams_noIcon() {
val adapter = StreamItemAdapter<SubtitlesStream, Stream>(
StreamItemAdapter.StreamSizeWrapper(
StreamItemAdapter.StreamInfoWrapper(
(0 until 5).map {
SubtitlesStream.Builder()
.setContent("https://example.com", true)
Expand All @@ -105,7 +111,7 @@ class StreamItemAdapterTest {
@Test
fun audioStreams_noIcon() {
val adapter = StreamItemAdapter<AudioStream, Stream>(
StreamItemAdapter.StreamSizeWrapper(
StreamItemAdapter.StreamInfoWrapper(
(0 until 5).map {
AudioStream.Builder()
.setId(Stream.ID_UNKNOWN)
Expand All @@ -123,12 +129,109 @@ class StreamItemAdapterTest {
}
}

@Test
fun retrieveMediaFormatFromFileTypeHeaders() {
val streams = getIncompleteAudioStreams(5)
val wrapper = StreamInfoWrapper(streams, context)
val retrieveMediaFormat = { stream: AudioStream, response: Response ->
StreamInfoWrapper.retrieveMediaFormatFromFileTypeHeaders(stream, wrapper, response)
}
val helper = AssertionHelper(streams, wrapper, retrieveMediaFormat)

helper.assertInvalidResponse(getResponse(mapOf(Pair("content-length", "mp3"))), 0)
helper.assertInvalidResponse(getResponse(mapOf(Pair("file-type", "mp0"))), 1)

helper.assertValidResponse(getResponse(mapOf(Pair("x-amz-meta-file-type", "aiff"))), 2, MediaFormat.AIFF)
helper.assertValidResponse(getResponse(mapOf(Pair("file-type", "mp3"))), 3, MediaFormat.MP3)
}

@Test
fun retrieveMediaFormatFromContentDispositionHeader() {
val streams = getIncompleteAudioStreams(11)
val wrapper = StreamInfoWrapper(streams, context)
val retrieveMediaFormat = { stream: AudioStream, response: Response ->
StreamInfoWrapper.retrieveMediaFormatFromContentDispositionHeader(stream, wrapper, response)
}
val helper = AssertionHelper(streams, wrapper, retrieveMediaFormat)

helper.assertInvalidResponse(getResponse(mapOf(Pair("content-length", "mp3"))), 0)
helper.assertInvalidResponse(
getResponse(mapOf(Pair("Content-Disposition", "filename=\"train.png\""))), 1
)
helper.assertInvalidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"data.csv\""))), 2
)
helper.assertInvalidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; filename=\"data.csv\""))), 3
)
helper.assertInvalidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"fieldName\"; filename*=\"filename.jpg\""))), 4
)

helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "filename=\"train.ogg\""))),
5, MediaFormat.OGG
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "some-form-data; filename=\"audio.flac\""))),
6, MediaFormat.FLAC
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"audio.aiff\"; filename=\"audio.aiff\""))),
7, MediaFormat.AIFF
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"alien?\"; filename*=UTF-8''%CE%B1%CE%BB%CE%B9%CF%B5%CE%BD.m4a"))),
8, MediaFormat.M4A
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"audio.mp3\"; filename=\"audio.opus\"; filename*=UTF-8''alien.opus"))),
9, MediaFormat.OPUS
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Disposition", "form-data; name=\"audio.mp3\"; filename=\"audio.opus\"; filename*=\"UTF-8''alien.opus\""))),
10, MediaFormat.OPUS
)
}

@Test
fun retrieveMediaFormatFromContentTypeHeader() {
val streams = getIncompleteAudioStreams(12)
val wrapper = StreamInfoWrapper(streams, context)
val retrieveMediaFormat = { stream: AudioStream, response: Response ->
StreamInfoWrapper.retrieveMediaFormatFromContentTypeHeader(stream, wrapper, response)
}
val helper = AssertionHelper(streams, wrapper, retrieveMediaFormat)

helper.assertInvalidResponse(getResponse(mapOf(Pair("content-length", "984501"))), 0)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "audio/xyz"))), 1)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "mp3"))), 2)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "mp3"))), 3)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "audio/mpeg"))), 4)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "audio/aif"))), 5)
helper.assertInvalidResponse(getResponse(mapOf(Pair("Content-Type", "whatever"))), 6)
helper.assertInvalidResponse(getResponse(mapOf()), 7)

helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Type", "audio/flac"))), 8, MediaFormat.FLAC
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Type", "audio/wav"))), 9, MediaFormat.WAV
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Type", "audio/opus"))), 10, MediaFormat.OPUS
)
helper.assertValidResponse(
getResponse(mapOf(Pair("Content-Type", "audio/aiff"))), 11, MediaFormat.AIFF
)
}

/**
* @return a list of video streams, in which their video only property mirrors the provided
* [videoOnly] vararg.
*/
private fun getVideoStreams(vararg videoOnly: Boolean) =
StreamItemAdapter.StreamSizeWrapper(
StreamItemAdapter.StreamInfoWrapper(
videoOnly.map {
VideoStream.Builder()
.setId(Stream.ID_UNKNOWN)
Expand Down Expand Up @@ -161,6 +264,19 @@ class StreamItemAdapterTest {
}
)

private fun getIncompleteAudioStreams(size: Int): List<AudioStream> {
val list = ArrayList<AudioStream>(size)
for (i in 1..size) {
list.add(
AudioStream.Builder()
.setId(Stream.ID_UNKNOWN)
.setContent("https://example.com/$i", true)
.build()
)
}
return list
}

/**
* Checks whether the item at [position] in the [spinner] has the correct icon visibility when
* it is shown in normal mode (selected) and in dropdown mode (user is choosing one of a list).
Expand Down Expand Up @@ -196,11 +312,56 @@ class StreamItemAdapterTest {
streams.forEachIndexed { index, stream ->
val secondaryStreamHelper: SecondaryStreamHelper<T>? = stream?.let {
SecondaryStreamHelper(
StreamItemAdapter.StreamSizeWrapper(streams, context),
StreamItemAdapter.StreamInfoWrapper(streams, context),
it
)
}
put(index, secondaryStreamHelper)
}
}

private fun getResponse(headers: Map<String, String>): Response {
val listHeaders = HashMap<String, List<String>>()
headers.forEach { entry ->
listHeaders[entry.key] = listOf(entry.value)
}
return Response(200, null, listHeaders, "", "")
}

/**
* Helper class for assertion related to extractions of [MediaFormat]s.
*/
class AssertionHelper<T : Stream>(
private val streams: List<T>,
private val wrapper: StreamInfoWrapper<T>,
private val retrieveMediaFormat: (stream: T, response: Response) -> Boolean
) {

/**
* Assert that an invalid response does not result in wrongly extracted [MediaFormat].
*/
fun assertInvalidResponse(
response: Response,
index: Int
) {
assertFalse(
"invalid header returns valid value", retrieveMediaFormat(streams[index], response)
)
assertNull("Media format extracted although stated otherwise", wrapper.getFormat(index))
}

/**
* Assert that a valid response results in correctly extracted and handled [MediaFormat].
*/
fun assertValidResponse(
response: Response,
index: Int,
format: MediaFormat
) {
assertTrue(
"header was not recognized", retrieveMediaFormat(streams[index], response)
)
assertEquals("Wrong media format extracted", format, wrapper.getFormat(index))
}
}
}
34 changes: 17 additions & 17 deletions app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
import org.schabi.newpipe.util.SecondaryStreamHelper;
import org.schabi.newpipe.util.SimpleOnSeekBarChangeListener;
import org.schabi.newpipe.util.StreamItemAdapter;
import org.schabi.newpipe.util.StreamItemAdapter.StreamSizeWrapper;
import org.schabi.newpipe.util.StreamItemAdapter.StreamInfoWrapper;
import org.schabi.newpipe.util.AudioTrackAdapter;
import org.schabi.newpipe.util.AudioTrackAdapter.AudioTracksWrapper;
import org.schabi.newpipe.util.ThemeHelper;
Expand Down Expand Up @@ -97,9 +97,9 @@ public class DownloadDialog extends DialogFragment
@State
StreamInfo currentInfo;
@State
StreamSizeWrapper<VideoStream> wrappedVideoStreams;
StreamInfoWrapper<VideoStream> wrappedVideoStreams;
@State
StreamSizeWrapper<SubtitlesStream> wrappedSubtitleStreams;
StreamInfoWrapper<SubtitlesStream> wrappedSubtitleStreams;
@State
AudioTracksWrapper wrappedAudioTracks;
@State
Expand Down Expand Up @@ -187,8 +187,8 @@ public DownloadDialog(@NonNull final Context context, @NonNull final StreamInfo
wrappedAudioTracks.size() > 1
);

this.wrappedVideoStreams = new StreamSizeWrapper<>(videoStreams, context);
this.wrappedSubtitleStreams = new StreamSizeWrapper<>(
this.wrappedVideoStreams = new StreamInfoWrapper<>(videoStreams, context);
this.wrappedSubtitleStreams = new StreamInfoWrapper<>(
getStreamsOfSpecifiedDelivery(info.getSubtitles(), PROGRESSIVE_HTTP), context);

this.selectedVideoIndex = ListHelper.getDefaultResolutionIndex(context, videoStreams);
Expand Down Expand Up @@ -258,10 +258,10 @@ public void onServiceDisconnected(final ComponentName name) {
* Update the displayed video streams based on the selected audio track.
*/
private void updateSecondaryStreams() {
final StreamSizeWrapper<AudioStream> audioStreams = getWrappedAudioStreams();
final StreamInfoWrapper<AudioStream> audioStreams = getWrappedAudioStreams();
final var secondaryStreams = new SparseArrayCompat<SecondaryStreamHelper<AudioStream>>(4);
final List<VideoStream> videoStreams = wrappedVideoStreams.getStreamsList();
wrappedVideoStreams.resetSizes();
wrappedVideoStreams.resetInfo();

for (int i = 0; i < videoStreams.size(); i++) {
if (!videoStreams.get(i).isVideoOnly()) {
Expand Down Expand Up @@ -396,7 +396,7 @@ public void onSaveInstanceState(@NonNull final Bundle outState) {

private void fetchStreamsSize() {
disposables.clear();
disposables.add(StreamSizeWrapper.fetchSizeForWrapper(wrappedVideoStreams)
disposables.add(StreamInfoWrapper.fetchMoreInfoForWrapper(wrappedVideoStreams)
.subscribe(result -> {
if (dialogBinding.videoAudioGroup.getCheckedRadioButtonId()
== R.id.video_button) {
Expand All @@ -406,7 +406,7 @@ private void fetchStreamsSize() {
new ErrorInfo(throwable, UserAction.DOWNLOAD_OPEN_DIALOG,
"Downloading video stream size",
currentInfo.getServiceId()))));
disposables.add(StreamSizeWrapper.fetchSizeForWrapper(getWrappedAudioStreams())
disposables.add(StreamInfoWrapper.fetchMoreInfoForWrapper(getWrappedAudioStreams())
.subscribe(result -> {
if (dialogBinding.videoAudioGroup.getCheckedRadioButtonId()
== R.id.audio_button) {
Expand All @@ -416,7 +416,7 @@ private void fetchStreamsSize() {
new ErrorInfo(throwable, UserAction.DOWNLOAD_OPEN_DIALOG,
"Downloading audio stream size",
currentInfo.getServiceId()))));
disposables.add(StreamSizeWrapper.fetchSizeForWrapper(wrappedSubtitleStreams)
disposables.add(StreamInfoWrapper.fetchMoreInfoForWrapper(wrappedSubtitleStreams)
.subscribe(result -> {
if (dialogBinding.videoAudioGroup.getCheckedRadioButtonId()
== R.id.subtitle_button) {
Expand Down Expand Up @@ -724,9 +724,9 @@ private void setRadioButtonsState(final boolean enabled) {
dialogBinding.subtitleButton.setEnabled(enabled);
}

private StreamSizeWrapper<AudioStream> getWrappedAudioStreams() {
private StreamInfoWrapper<AudioStream> getWrappedAudioStreams() {
if (selectedAudioTrackIndex < 0 || selectedAudioTrackIndex > wrappedAudioTracks.size()) {
return StreamSizeWrapper.empty();
return StreamInfoWrapper.empty();
}
return wrappedAudioTracks.getTracksList().get(selectedAudioTrackIndex);
}
Expand Down Expand Up @@ -766,7 +766,7 @@ private String getNameEditText() {
}

private void showFailedDialog(@StringRes final int msg) {
assureCorrectAppLanguage(getContext());
assureCorrectAppLanguage(requireContext());
new AlertDialog.Builder(context)
.setTitle(R.string.general_error)
.setMessage(msg)
Expand Down Expand Up @@ -799,7 +799,7 @@ private void prepareSelectedDownload() {
filenameTmp += "opus";
} else if (format != null) {
mimeTmp = format.mimeType;
filenameTmp += format.suffix;
filenameTmp += format.getSuffix();
}
break;
case R.id.video_button:
Expand All @@ -808,7 +808,7 @@ private void prepareSelectedDownload() {
format = videoStreamsAdapter.getItem(selectedVideoIndex).getFormat();
if (format != null) {
mimeTmp = format.mimeType;
filenameTmp += format.suffix;
filenameTmp += format.getSuffix();
}
break;
case R.id.subtitle_button:
Expand All @@ -820,9 +820,9 @@ private void prepareSelectedDownload() {
}

if (format == MediaFormat.TTML) {
filenameTmp += MediaFormat.SRT.suffix;
filenameTmp += MediaFormat.SRT.getSuffix();
} else if (format != null) {
filenameTmp += format.suffix;
filenameTmp += format.getSuffix();
}
break;
default:
Expand Down
8 changes: 4 additions & 4 deletions app/src/main/java/org/schabi/newpipe/util/AudioTrackAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

import org.schabi.newpipe.R;
import org.schabi.newpipe.extractor.stream.AudioStream;
import org.schabi.newpipe.util.StreamItemAdapter.StreamSizeWrapper;
import org.schabi.newpipe.util.StreamItemAdapter.StreamInfoWrapper;

import java.io.Serializable;
import java.util.List;
Expand Down Expand Up @@ -75,15 +75,15 @@ public View getView(final int position, final View convertView, final ViewGroup
}

public static class AudioTracksWrapper implements Serializable {
private final List<StreamSizeWrapper<AudioStream>> tracksList;
private final List<StreamInfoWrapper<AudioStream>> tracksList;

public AudioTracksWrapper(@NonNull final List<List<AudioStream>> groupedAudioStreams,
@Nullable final Context context) {
this.tracksList = groupedAudioStreams.stream().map(streams ->
new StreamSizeWrapper<>(streams, context)).collect(Collectors.toList());
new StreamInfoWrapper<>(streams, context)).collect(Collectors.toList());
}

public List<StreamSizeWrapper<AudioStream>> getTracksList() {
public List<StreamInfoWrapper<AudioStream>> getTracksList() {
return tracksList;
}

Expand Down
6 changes: 3 additions & 3 deletions app/src/main/java/org/schabi/newpipe/util/SecondaryStreamHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
import org.schabi.newpipe.extractor.stream.AudioStream;
import org.schabi.newpipe.extractor.stream.Stream;
import org.schabi.newpipe.extractor.stream.VideoStream;
import org.schabi.newpipe.util.StreamItemAdapter.StreamSizeWrapper;
import org.schabi.newpipe.util.StreamItemAdapter.StreamInfoWrapper;

import java.util.List;

public class SecondaryStreamHelper<T extends Stream> {
private final int position;
private final StreamSizeWrapper<T> streams;
private final StreamInfoWrapper<T> streams;

public SecondaryStreamHelper(@NonNull final StreamSizeWrapper<T> streams,
public SecondaryStreamHelper(@NonNull final StreamInfoWrapper<T> streams,
final T selectedStream) {
this.streams = streams;
this.position = streams.getStreamsList().indexOf(selectedStream);
Expand Down
Loading
Loading