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

[MediaCCC] Allow obtaining channel tab link handler #1148

Merged
merged 3 commits into from
Mar 28, 2024

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Dec 30, 2023

This is a proper fix for TeamNewPipe/NewPipe#10698. Now it is possible to obtain a channel tab link handler, and null is never returned. The comments scattered around the code should explain my approach better than I could do in the PR description ;-), but the important point is that I reused the existing channel link handler for channel tabs, too.

Fixes TransactionTooLargeException when switching away from NewPipe while there are MediaCCC channels open. Before I could reproduce this consistently, while with this PR the crash is fixed.

The fact that the channel and the channel tab have the same url made it so that NewPipe would cache them both under the same key. I opened TeamNewPipe/NewPipe#10717 to fix that, and you can try the APK there to test these changes.

i.e. without needing to pass through the conference/channel extractor
This was needed because clients (like NewPipe) might rely on link handlers to hold as little data as possible, since they might be kept around for long or passed around in system transactions, so this commit allows obtaining a standalone link handler that does not hold a JsonObject within itself.
This caused problems in NewPipe, because extractors are not serializable, and well, keeping references to them is a bad idea anyway.
@TobiGr
Copy link
Member

TobiGr commented Dec 31, 2023

I guess this is a fix for TeamNewPipe/NewPipe#10714, isn't it?

@TobiGr TobiGr added the media.ccc.de service, https://media.ccc.de label Dec 31, 2023
@Stypox
Copy link
Member Author

Stypox commented Dec 31, 2023

Probably it is, although the log posted by the user was fixed in your hotfix PR

@TobiGr
Copy link
Member

TobiGr commented Dec 31, 2023

Ah, I just saw that the user listed 0.26.1 as affected version, but the logs indicate that the crash was happening on 0.26.0...

@Stypox Stypox merged commit 6589e2c into TeamNewPipe:dev Mar 28, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
media.ccc.de service, https://media.ccc.de
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants