Skip to content

Conversation

@yausername
Copy link
Contributor

@yausername yausername commented Jul 1, 2020

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Requires #358
Closes #304

Some videos, especially those that have low views, don't have the usual
direct links to all streams, this fix it by properly parsing the DASH
manifest from those special cases.
Rely on the DASH manifest streams instead.
@TobiGr TobiGr added BBC Sounds new service Issues (feature request) or PR related to a new service labels Jul 1, 2020
@Stypox
Copy link
Member

Stypox commented Mar 17, 2021

@Isira-Seneviratne you can make a single review out of multiple code comments instead of a different one for each comment

@Isira-Seneviratne
Copy link
Member

@Isira-Seneviratne you can make a single review out of multiple code comments instead of a different one for each comment

Okay, I will.

Comment on lines +77 to +78
//noinspection EqualsReplaceableByObjectsCall
return url != null ? url.equals(hls.url) : hls.url == null;
Copy link
Member

Choose a reason for hiding this comment

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

Since Java 8 support has been enabled in NewPipe, Objects.equals() will be backported for API levels below 19.

Suggested change
//noinspection EqualsReplaceableByObjectsCall
return url != null ? url.equals(hls.url) : hls.url == null;
return Objects.equals(url, direct.url);


@Override
public int hashCode() {
return url != null ? url.hashCode() : 0;
Copy link
Member

Choose a reason for hiding this comment

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

Same for Objects.hashCode().

Suggested change
return url != null ? url.hashCode() : 0;
return Objects.hashCode(url);

Comment on lines +116 to +124
//noinspection EqualsReplaceableByObjectsCall
if (baseUrl != null ? !baseUrl.equals(that.baseUrl) : that.baseUrl != null) {
return false;
}

//noinspection EqualsReplaceableByObjectsCall
return manualDashManifest != null
? manualDashManifest.equals(that.manualDashManifest)
: that.manualDashManifest == null;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//noinspection EqualsReplaceableByObjectsCall
if (baseUrl != null ? !baseUrl.equals(that.baseUrl) : that.baseUrl != null) {
return false;
}
//noinspection EqualsReplaceableByObjectsCall
return manualDashManifest != null
? manualDashManifest.equals(that.manualDashManifest)
: that.manualDashManifest == null;
return Objects.equals(baseUrl, that.baseUrl) && Objects.equals(manualDashManifest, that.manualDashManifest);

Comment on lines +129 to +130
int result = baseUrl != null ? baseUrl.hashCode() : 0;
result = 31 * result + (manualDashManifest != null ? manualDashManifest.hashCode() : 0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int result = baseUrl != null ? baseUrl.hashCode() : 0;
result = 31 * result + (manualDashManifest != null ? manualDashManifest.hashCode() : 0);
int result = Objects.hashCode(baseUrl);
result = 31 * result + Objects.hashCode(manualDashManifest);

Comment on lines +46 to +47
//noinspection EqualsReplaceableByObjectsCall
return url != null ? url.equals(direct.url) : direct.url == null;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//noinspection EqualsReplaceableByObjectsCall
return url != null ? url.equals(direct.url) : direct.url == null;
return Objects.equals(url, direct.url);


@Override
public int hashCode() {
return url != null ? url.hashCode() : 0;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return url != null ? url.hashCode() : 0;
return Objects.hashCode(url);

@nadiration
Copy link

@yausername thank you very much, BBC Sounds would be a great addition. What's your status are you still working on it?

@yausername
Copy link
Contributor Author

Hey, Unfortunately I can't find time. I would be happy to review if someone else wants to take this PR.

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

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

uhm. Why did you use kotlin? We decided to keep the extractor kotlin-free / Java-only to help beginners to easily get into extending and fixing the extractor.

@yausername
Copy link
Contributor Author

uhm. Why did you use kotlin? We decided to keep the extractor kotlin-free / Java-only to help beginners to easily get into extending and fixing the extractor.

Hey @TobiGr
I wasn't aware of that. But I get why we would want to keep it java only.
I'm closing this PR for now.
If someone wanted they could use this code with some kotlin-java converter.
But even writing the extractor from scratch is easy enough.

If nothing happens I'll take it up when I'm available.
Love NewPipe :)

@yausername yausername closed this Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new service Issues (feature request) or PR related to a new service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Service request] bbc.co.uk/sounds

6 participants