Skip to content

Commit

Permalink
Merge pull request #10165 from TeamNewPipe/fix/media-format
Browse files Browse the repository at this point in the history 
Fix downloads of streams with missing MediaFormat
  • Loading branch information
@Stypox
Stypox authored Sep 19, 2023
2 parents 9e353f1 + 992bb5d commit 725c18e
Show file tree
Hide file tree
Showing 5 changed files with 373 additions and 52 deletions.
169 changes: 165 additions & 4 deletions app/src/androidTest/java/org/schabi/newpipe/util/StreamItemAdapterTest.kt
View file
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
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
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
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

0 comments on commit 725c18e

Please sign in to comment.