siku2 / script.service.sponsorblock

Kodi add-on for SponsorBlock
MIT License
124 stars 14 forks source link

feat: support get segments with hash #46

Closed SethFalco closed 1 year ago

SethFalco commented 1 year ago

Supports the endpoint to get video segments with the first 4 characters of a video SHA256 hash.

More info: https://wiki.sponsor.ajay.app/w/API_Docs#GET_/api/skipSegments/:sha256HashPrefix

Notes

Weird thing to look out for, but over here: https://github.com/SethFalco/script.service.sponsorblock/blob/279d9e8da5a8ba9be70e7b4bfd62bea532b12560/resources/lib/sponsorblock/api.py#L111

Despite the types suggesting it's a str, it's actually a list[str]. I was trying to figure out why, but it's actually a list[str] from all the way up from PlayerListener#_prepare_segments, so I thought I'd leave it for now.

siku2 commented 1 year ago

Despite the types suggesting it's a str, it's actually a list[str]. I was trying to figure out why, but it's actually a list[str] from all the way up from PlayerListener#_prepare_segments, so I thought I'd leave it for now.

That seems like a bug to me.

https://github.com/siku2/script.service.sponsorblock/blob/20569b5d65be6e4f8cf9a874625b8985e1407756/resources/lib/youtube_api.py#L157

All other paths in this function return a single string, but the query.get function returns a list, even though we only care about a single item. I'm actually surprised that this didn't manifest into an issue earlier.

SethFalco commented 1 year ago

No problem, I'll resolve this tonight! I'll also review failed to parse notification payload log from https://github.com/siku2/script.service.sponsorblock/pull/45 as well and put it on another PR.

SethFalco commented 1 year ago

but the query.get function returns a list, even though we only care about a single item.

Looked into this now, nice catch! Indeed that was the problem, the signature of #get was list[str] | None.

(method) def get(
    __key: str,
    /
) -> (list[str] | None)

Return the value for key if key is in the dictionary, else default.

It should be cool now, but only tested with Invidious.