[SoundCloud] Fix SoundCloud HLS expiry

[absurdlylongusername]

• 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.

Fix SoundCloud HLS stream urls expiring

This PR implements extractor side changes to facilitate refreshing expires HLS stream URLs for SoundCloud streams.

Adds some logging in some places as well

Please Note

Includes changes from:

• Validate HTTP response codes across the extractor  #1322
• [SoundCloud] Miscellaneous refactors #1323
• Add logging to extractor #1403

So those PRs must be merged before this.

See TeamNewPipe/NewPipe#12418 for the full writeup for the fix

[Stypox]

Thank you! My review gives some thoughts that we should decide on before continuing. There definitely needs to be a way to refresh streams in the extractor, which will come in handy not only for SoundCloud but also for YouTube.

I think it is a good idea to store refresh information in the streams themselves, and to make them expose methods that allow refreshing the stream. I guess this would also play well with #858 (comment). What do you think @AudricV?

Regarding logging, if I understand correctly the commit structure, it should be possible to extract it in a separate PR, that we can merge much faster. Could you put the logging in a separate PR? Also the

[Stypox]

Suggested change

	@SuppressWarnings("checkstyle:LeftCurly")
	public interface RefreshableStream {
	public interface RefreshableStream {

[Stypox]

Is this file used anywhere? Also, the reason why services are not implemented as an enum is also because it should be possible to dynamically add service implementations (think e.g. of plugins). Obviously this has never happened yet, but iirc that's why the API is like that.

[absurdlylongusername]

It's used in the NewPipe PR for logging the names of services.

It's possible to have enums and still allow for dynamic implementations. Since plugins are a long way away, I think it makes a lot more sense to have enums for each service to improve readability, traceability, logging and type safety.

Howbeit, I'm not going to change the entire API to use enums because that's long: I just want the nice id -> name static method.

[Stypox]

Can't you use this utility method instead? https://github.com/TeamNewPipe/NewPipe/blob/2a9c6f05387866bef2c42ab67226c14936917d27/app/src/main/java/org/schabi/newpipe/util/ServiceHelper.java#L138

But yeah I agree this is not the best. We should not have the concept of service ID at all except for saving stuff to database, so every StreamInfo should not have any getServiceId() method but rather getService() directly. So we don't have to keep switching back and forth between int and Service

[Stypox]

I don't think it's a property of all HLS streams that they are refreshable. I think we have two alternative ways we can handle this instead:

• RefreshableStream could just be an interface that some Streams can implement, then each Service would override VideoStream/AudioStream and also implement RefreshableStream if needed, and the player could check for refreshable streams using instanceof. This would require a lot of boilerplate though.
• Add a (Audio/Video)Stream refresh() {} method to the base Stream class which builds a new refreshed stream, and make it return null if refreshing is not available.

[Stypox]

Shouldn't this also somehow store the expiry time? So that the player can prepare in advance if the stream is about to expire

[absurdlylongusername]

How exactly would the player prepare in advance? What is it supposed to do?

[Stypox]

If the currently playing stream expires at 18:50:45 and now it's 18:50:35, there are just 10 more seconds left to fetch data before needing to switch over to the refreshed stream. So instead of waiting until the URL returns 403 and then quitting, the player can notice that only 10 seconds are left and already pre-fetch the refreshed stream and switch to it as soon as it's ready.

[absurdlylongusername]

ExoPlayer handles buffering of stream data, so I don't think is necessary since stream chunks are already pre-fetched ahead of time.

It fetches chunks ahead of time, so when it gets a 403 it's not for the currently playing chunk.

[Stypox]

Oh right, that's true. I'd still add a nullable expiration Instant just in case other clients need it (or maybe NewPipe's Downloader might need it? Or maybe streams in other services that don't throw 403 and where it's more difficult to detect that the stream is now invalid?), since this is information the extractor usually knows. But this is not totally necessary, so do it only if it seems simple to extract such data.

[absurdlylongusername]

@Stypox

Regarding logging, if I understand correctly the commit structure, it should be possible to extract it in a separate PR, that we can merge much faster. Could you put the logging in a separate PR? Also the

Seems you didn't finish your sentence there lol.

Yeah sure I can make a separate logging PR that can be merged and then rebase this PR on top of it

[Stypox]

I have no idea what I wanted to say now, sorry xD

[absurdlylongusername]

@Stypox I've now created a separate PR for logging which this PR depends on: #1403