[YouTube] Mark members-only videos#1280
Conversation
There was a problem hiding this comment.
Thinking about it a bit more, maybe it makes sense to rename requiresMembership to isPaidContent (or something similar)? That would keep the naming consistent with the PaidContentException that is already thrown for member only content, and it would be generic enough for other services to use. But I'm not sure if that might conflict with something else, e.g. premium videos?
YouTube has started to mix members-only videos into the normal videos in the Videos tab. This filters them, as they cannot be watched without an account, leading to clutter in the feed. Requires TeamNewPipe/NewPipeExtractor#1280. Closes: TeamNewPipe#12011 Closes: TeamNewPipe#12040
YouTube has started to mix members-only videos into the normal videos in the Videos tab. This filters them, as they cannot be watched without an account, leading to clutter in the feed. Requires TeamNewPipe/NewPipeExtractor#1280. Closes: TeamNewPipe#12011 Closes: TeamNewPipe#12040
|
What about tests? Is it possible to create tests for the new extractions? |
|
I'm honestly a bit overwhelmed with how to create tests for static class MembersOnlyVideos extends DefaultListExtractorTest<ChannelTabExtractor> {
private static YoutubeChannelTabExtractor extractor;
@BeforeAll
static void setUp() throws IOException, ExtractionException {
YoutubeTestsUtils.ensureStateless();
NewPipe.init(DownloaderFactory.getDownloader(RESOURCE_PATH + "membersOnlyVideos"));
extractor = (YoutubeChannelTabExtractor) YouTube.getChannelTabExtractorFromId(
"@TheLinuxEXP", ChannelTabs.VIDEOS);
extractor.fetchPage();
}
@Override public ChannelTabExtractor extractor() throws Exception { return extractor; }
@Override public StreamingService expectedService() throws Exception { return YouTube; }
@Override public String expectedName() throws Exception { return ChannelTabs.VIDEOS; }
@Override public String expectedId() throws Exception { return "UC5UAwBUum7CPN5buc-_N1Fw"; }
@Override public String expectedUrlContains() throws Exception { return "https://www.youtube.com/channel/UC5UAwBUum7CPN5buc-_N1Fw/videos"; }
@Override public String expectedOriginalUrlContains() throws Exception { return "https://www.youtube.com/@TheLinuxEXP/videos"; }
@Override public boolean expectedHasMoreItems() { return true; }
@Test
@Override
public void testRelatedItems() throws Exception {
final List<StreamInfoItem> allItems = extractor.getInitialPage().getItems()
.stream()
.filter(StreamInfoItem.class::isInstance)
.map(StreamInfoItem.class::cast)
.collect(Collectors.toUnmodifiableList());
final List<StreamInfoItem> membershipVideos = allItems.stream()
.filter(item -> !item.requiresMembership())
.collect(Collectors.toUnmodifiableList());
assertTrue(membershipVideos.size() <= allItems.size());
}
}logAnd just checking if there are |
|
Note about tests: When writing a channel extractor test the channel should be chosen very carefully. I think it's best to chose a channel here that either
Maybe as an alternative a playlist can also be considered. |
|
Added a test using a playlist with only members only videos. |
TobiGr
left a comment
There was a problem hiding this comment.
LGTM, please add the two jdoc comments
|
For #1280 (review), you're right, a better approach would be to create an enum that would return availability type (creator membership required, service paid content, upcoming content, available content, geo-restricted, ...), it could be applied to info items and items. This would solve several issues of detecting some content unavailability type in a good way in the extractor. |
|
As suggested in #1280 (comment), I rewrote the code to return an enum with different availabilities instead. I left out the test for the other types for now. Also, if I remember correctly, for upcoming videos an exception is thrown, so it wouldn't be possible to use the enum, but I would gladly leave this to someone with more experience in the code base :) |
|
@AudricV One general question: should the |
AudricV
left a comment
There was a problem hiding this comment.
It looks almost good, thank you!
2041b3c to
32cdee8
Compare
I don't think so. For me, availability and privacy (or better for this term visibility), should be different values.
Except my very nitpicking change request which has been not applied, it could make sense to add a new value describing we don't have any info for the availability like Changes LGTM otherwise (unless we want better docs), we need to apply these changes to Soundcloud (and Bandcamp too I think) tracks :) |
9a0f4a8 to
291946d
Compare
291946d to
7684059
Compare
|
Regenerated the mocks and squashed them into ef0da2b. |
YouTube inserts members-only videos (i.e., videos that require channel membership to watch) into the Videos tab. Because the extractor unable to distinguish between them and "normal" videos, they may appear in subscriptions feeds. This enables the extractor to check if videos require membership, allowing clients to filter them. Ref: TeamNewPipe/NewPipe#12040 Ref: TeamNewPipe/NewPipe#12011
Adds a new field to `StreamInfoItem` to denote the availability of the stream. A stream may be restricted to only certain user groups or times. This allows users to ignore the stream based on their availability, e.g. a app may choose to hide all streams that require payment.
7684059 to
9b98978
Compare
Stypox
left a comment
There was a problem hiding this comment.
Looks good to me, thanks! I pushed two commit, one to add the UNKNOWN field as suggested by Audric, and another to add a base test for StreamInfo that currently does nothing useful (since StreamInfo.getContentAvailability() always returns UNKNOWN), but that will force any future contributors that want to implement overrides for StreamInfo.getContentAvailability() to also implement tests.
YouTube inserts members-only videos (i.e., videos that require channel membership to watch) into the Videos tab. Because the extractor unable to distinguish between them and "normal" videos, they may appear in subscriptions feeds.
This enables the extractor to check if videos require membership, allowing clients to filter them.
Ref: TeamNewPipe/NewPipe#12040
Ref: TeamNewPipe/NewPipe#12011