-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
plugins.twitch: filter ads by EXT-X-DATERANGE tag #3213
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
plugins.twitch: filter ads by EXT-X-DATERANGE tag #3213
Conversation
|
I'm still seeing the following error from a random "big" stream pulled off twitch: |
b763839 to
40af3f4
Compare
Why don’t you use the isodate module for parsing, which is also used for the dash manifest parser? |
|
I'm getting same errors as gravyboat, but it did work once (line 62) but I might be doing this wrong but here's my output. Let me know if I should adjust something: https://gist.github.com/azizLIGHT/0fce5e83961a007fbae03dbd39eb6a0b |
40af3f4 to
f02d419
Compare
|
@Billy2011 Thanks... Don't ask how I've missed that. 😞 |
I got the latest version and tried again but got same errors about |
And add support for the parser to return a custom M3U8 class
f02d419 to
c907c73
Compare
|
@bastimeyer Most recent changes look good on a quick test. I get the pre-roll notification on the CLI then the stream opens correctly once they're done. |
|
@bastimeyer Yes there anything else you want to add here? Otherwise I want to squash and merge. |
No, that should be pretty much it. As said in the OP, some tag attributes haven't been implemented, but that doesn't matter. One thing to note though is that I've also ignored the But before merging, I would prefer if we could get some feedback from other users as well, because I still don't get ads on Twitch from my IP address and have basically implemented something purely on the HLS spec and the reported playlist contents, and therefore don't know if I have missed anything in regards to the ad-daterange detection.
Don't squash. Both commits change two different components and therefore shouldn't be squashed (and they also have proper commit messages). Either rebase onto master or create a merge commit. |
c907c73 to
6f89477
Compare
|
@gravyboat I think we can merge this. Should be fine. Don't squash (two commits affecting two different components) |
Original PR #3213 by bastimeyer Original: streamlink/streamlink#3213
Merged from original PR #3213 Original: streamlink/streamlink#3213
As mentioned in #3210 (comment), simply filtering ads by comparing each segment's title isn't good enough anymore and checking for the title not being equal to "live" can break the implementation if Twitch decides to change the title of live stream segments or if they decide to add titles to VOD segments.
So here's a better filtering approach, based on the
EXT-X-DATERANGEandEXT-X-PROGRAM-DATE-TIMEtags.Example playlists can be found here:
Basic description of each commit:
Adds the
EXT-X-DATERANGEtag to the HLS playlist parser and makes theEXT-X-PROGRAM-DATE-TIMEtag return a datetime object instead of a string. Since I couldn't find any usage of theEXT-X-PROGRAM-DATE-TIMEtag in the existing code, changing its return value should be fine. Also adds support for returning a custom M3U8 class by the parser and adds ais_date_in_daterangemethod to the existingM3U8class. This isn't an ideal solution, but I couldn't find a better place for this method so that it can be called when parsing is done apart from turning theSegmentnamedtuple into a regular class. Also worth to note is that I haven't implemented all possible attribute values of theEXT-X-DATERANGE tag, because I didn't need them (scte35 attributes).Makes the Twitch plugin filter the parsed dateranges by attributes. It tries to find "ad dateranges" by the values of the ID and CLASS attributes and key names of custom attributes by Twitch. This is the crucial part of the ad detection. If a segment is then part of this daterange, it is marked as an ad and filtered out by the existing methods.
I've added tests for all "ad daterange" values I know of, but since I still don't get any ads by Twitch, it's hard to know if my implementation is working correctly. So please go ahead and test this on live data. Thank you!