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.

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

Do not list YouTube's OTF streams #400

Merged
merged 1 commit into from
Sep 29, 2020
Merged

Do not list YouTube's OTF streams #400

merged 1 commit into from
Sep 29, 2020

Conversation

wb9688
Copy link
Contributor

@wb9688 wb9688 commented Sep 29, 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.

Ignore streams that are delivered using YouTube's OTF format, as those only work with DASH and not with progressive HTTP.

@Stypox
Copy link
Member

Stypox commented Sep 29, 2020

Does this remove the streams causing infinite loading?

@wb9688
Copy link
Contributor Author

wb9688 commented Sep 29, 2020

@Stypox: Yes, that's what this PR is for.

@Stypox
Copy link
Member

Stypox commented Sep 29, 2020

@TobiGr I'd merge this in 0.20.0 to bring better stability, it is such a small change

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Do you think we should add a test for this? Maybe we can do without, if you tested well, since this code is going to be removed anyway in the future

@TobiGr
Copy link
Contributor

TobiGr commented Sep 29, 2020

Stuff like this should be done in the frontend and not the extractor. We can merge this now, but IMO we should extract all itags and provide all stream formats through the extractor. The frontend needs to sort out unsupported formats.

@wb9688
Copy link
Contributor Author

wb9688 commented Sep 29, 2020

@TobiGr: No. Currently NPE is supposed to only return progressive HTTP streams (and sometimes a torrent as well). These streams aren't progressive HTTP, so NPE shouldn't return them. There is also not really a proper way how NP could figure that out. With my DASH/HLS support PR, there will be a way to specify what kind of streaming method should be used, and then a frontend should skip e.g. DASH streams if they don't support that. However that's not the case yet. After I finish that PR and that gets merged, I'll make a PR extracting all itags based on the metadata that YouTube provides, which could include e.g. 360° videos, which should then also be filtered out by frontends not supporting that.

@TobiGr TobiGr changed the title Skip YouTube's OTF streams Do not list YouTube's OTF streams Sep 29, 2020
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.

Looks good to me.
I do not have time to write tests for this PR, @wb9688 hasn't either. Let's merge this and wait until @wb9688 finishes #367

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants