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

AudioStream: remove unused youtube-specific fields #1150

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

AudioStream: remove unused youtube-specific fields #1150

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
112 changes: 4 additions & 108 deletions extractor/src/main/java/org/schabi/newpipe/extractor/stream/AudioStream.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,6 @@ public final class AudioStream extends Stream {

private final int averageBitrate;

// Fields for DASH
private int itag = ITAG_NOT_AVAILABLE_OR_NOT_APPLICABLE;
private int bitrate;
private int initStart;
private int initEnd;
private int indexStart;
private int indexEnd;
private String quality;
private String codec;
Comment on lines -36 to -44
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I was the one who added these fields originally, they are used in Piped's backend for providing sidX boxes for streaming with DASH.

Here's how an audio representation is created by a client: https://github.com/TeamPiped/Piped/blob/8ee2a1c20737ce9db84186e00849b405d6c097af/src/utils/DashUtils.js#L99-L117

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then maybe a comment should be added that these were added for Piped? Similar confusion can take place whenever someone compares NP and NPE code in the future.


// Fields about the audio track id/name
@Nullable
private final String audioTrackId;
Expand All @@ -53,6 +43,7 @@ public final class AudioStream extends Stream {
@Nullable
private final AudioTrackType audioTrackType;

// * Youtube-backend-specific data
@Nullable
private ItagItem itagItem;

Expand Down Expand Up @@ -307,8 +298,7 @@ public AudioStream build() {
}

return new AudioStream(id, content, isUrl, mediaFormat, deliveryMethod, averageBitrate,
manifestUrl, audioTrackId, audioTrackName, audioLocale, audioTrackType,
itagItem);
manifestUrl, audioTrackId, audioTrackName, audioLocale, audioTrackType);
}
}

Expand All @@ -329,7 +319,6 @@ public AudioStream build() {
* @param audioTrackId the id of the audio track
* @param audioTrackName the name of the audio track
* @param audioLocale the {@link Locale} of the audio stream, representing its language
* @param itagItem the {@link ItagItem} corresponding to the stream, which cannot be null
* @param manifestUrl the URL of the manifest this stream comes from (if applicable,
* otherwise null)
*/
Expand All @@ -344,20 +333,9 @@ private AudioStream(@Nonnull final String id,
@Nullable final String audioTrackId,
@Nullable final String audioTrackName,
@Nullable final Locale audioLocale,
@Nullable final AudioTrackType audioTrackType,
@Nullable final ItagItem itagItem) {
@Nullable final AudioTrackType audioTrackType
) {
super(id, content, isUrl, format, deliveryMethod, manifestUrl);
if (itagItem != null) {
this.itagItem = itagItem;
this.itag = itagItem.id;
this.quality = itagItem.getQuality();
this.bitrate = itagItem.getBitrate();
this.initStart = itagItem.getInitStart();
this.initEnd = itagItem.getInitEnd();
this.indexStart = itagItem.getIndexStart();
this.indexEnd = itagItem.getIndexEnd();
this.codec = itagItem.getCodec();
}
this.averageBitrate = averageBitrate;
this.audioTrackId = audioTrackId;
this.audioTrackName = audioTrackName;
Expand Down Expand Up @@ -386,88 +364,6 @@ public int getAverageBitrate() {
return averageBitrate;
}

/**
* Get the itag identifier of the stream.
*
* <p>
* Always equals to {@link #ITAG_NOT_AVAILABLE_OR_NOT_APPLICABLE} for other streams than the
* ones of the YouTube service.
* </p>
*
* @return the number of the {@link ItagItem} passed in the constructor of the audio stream.
*/
public int getItag() {
return itag;
}

/**
* Get the bitrate of the stream.
*
* @return the bitrate set from the {@link ItagItem} passed in the constructor of the stream.
*/
public int getBitrate() {
return bitrate;
}

/**
* Get the initialization start of the stream.
*
* @return the initialization start value set from the {@link ItagItem} passed in the
* constructor of the stream.
*/
public int getInitStart() {
return initStart;
}

/**
* Get the initialization end of the stream.
*
* @return the initialization end value set from the {@link ItagItem} passed in the constructor
* of the stream.
*/
public int getInitEnd() {
return initEnd;
}

/**
* Get the index start of the stream.
*
* @return the index start value set from the {@link ItagItem} passed in the constructor of the
* stream.
*/
public int getIndexStart() {
return indexStart;
}

/**
* Get the index end of the stream.
*
* @return the index end value set from the {@link ItagItem} passed in the constructor of the
* stream.
*/
public int getIndexEnd() {
return indexEnd;
}

/**
* Get the quality of the stream.
*
* @return the quality label set from the {@link ItagItem} passed in the constructor of the
* stream.
*/
public String getQuality() {
return quality;
}

/**
* Get the codec of the stream.
*
* @return the codec set from the {@link ItagItem} passed in the constructor of the stream.
*/
public String getCodec() {
return codec;
}

/**
* Get the id of the audio track.
*
Expand Down
30 changes: 22 additions & 8 deletions ...t/java/org/schabi/newpipe/extractor/services/youtube/YoutubeDashManifestCreatorsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.io.StringReader;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Random;
import java.util.function.Consumer;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -123,12 +124,17 @@ private void assertDashStreams(final List<? extends Stream> streams) throws Exce
for (final Stream stream : assertFilterStreams(streams, DeliveryMethod.DASH)) {
//noinspection ConstantConditions
final String manifest = YoutubeOtfDashManifestCreator.fromOtfStreamingUrl(
stream.getContent(), stream.getItagItem(), videoLength);
stream.getContent(),
// We know that itagItem has to be set, because it’s youtube-specific
Objects.requireNonNull(stream.getItagItem()),
videoLength
);
assertNotBlank(manifest);

assertManifestGenerated(
manifest,
stream.getItagItem(),
// We know that itagItem has to be set, because it’s youtube-specific
Objects.requireNonNull(stream.getItagItem()),
document -> assertAll(
() -> assertSegmentTemplateElement(document),
() -> assertSegmentTimelineAndSElements(document)
Expand All @@ -143,16 +149,22 @@ private void assertProgressiveStreams(final List<? extends Stream> streams) thro
//noinspection ConstantConditions
final String manifest =
YoutubeProgressiveDashManifestCreator.fromProgressiveStreamingUrl(
stream.getContent(), stream.getItagItem(), videoLength);
stream.getContent(),
// We know that itagItem has to be set, because it’s youtube-specific
Objects.requireNonNull(stream.getItagItem()),
videoLength);
assertNotBlank(manifest);

@Nonnull final ItagItem itagItem =
// We know that itagItem has to be set, because it’s youtube-specific
Objects.requireNonNull(stream.getItagItem());
assertManifestGenerated(
manifest,
stream.getItagItem(),
itagItem,
document -> assertAll(
() -> assertBaseUrlElement(document),
() -> assertSegmentBaseElement(document, stream.getItagItem()),
() -> assertInitializationElement(document, stream.getItagItem())
() -> assertSegmentBaseElement(document, itagItem),
() -> assertInitializationElement(document, itagItem)
)
);
}
Expand All @@ -171,15 +183,17 @@ private List<? extends Stream> assertFilterStreams(
assertAll(filteredStreams.stream()
.flatMap(stream -> java.util.stream.Stream.of(
() -> assertNotBlank(stream.getContent()),
() -> assertNotNull(stream.getItagItem())
() ->
// We know that itagItem has to be set, because it’s youtube-specific
assertNotNull(stream.getItagItem())
))
);

return filteredStreams;
}

private void assertManifestGenerated(final String dashManifest,
final ItagItem itagItem,
@Nonnull final ItagItem itagItem,
final Consumer<Document> additionalAsserts)
throws Exception {

Expand Down