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

Update most dependencies #4508

Closed
wants to merge 1 commit into from
Closed

Update most dependencies #4508

wants to merge 1 commit into from

Conversation

wb9688
Copy link
Contributor

@wb9688 wb9688 commented Oct 14, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Update all dependencies except ExoPlayer. Most changes come from upgrading to RxJava 3, which unfortunately requires upgrading to an alpha version of Room, and Ktlint rules changes.

Note how I've added 3 dependencies: androidx.documentfile:documentfile and androidx.localbroadcastmanager:localbroadcastmanager were previously part of some other AndroidX library, while com.squareup.leakcanary:plumber-android is a new feature from LeakCanary that fixes known memory leaks in the Android framework.

I did not upgrade ExoPlayer, as we probably want to use certain new features like audio offloading, Media2 (replacement for MediaSessionCompat, which is now deprecated) support, and maybe toplevel playlist support. @Redirion: Do you want to do that in some PR?

Fixes the following issue(s)

Snyk complaining about vulnerabilities in some dependency. Edit: apparently it does not fix that for some reason.

Testing apk

app-debug.apk.zip

Agreement

@wb9688
Copy link
Contributor Author

wb9688 commented Oct 14, 2020

@opusforlife2: Could you test that everything still works? I'm mainly interested in if database import (please also test a database export from an old NewPipe version), subscription export, subscription import (especially its notification), and Markdown descriptions (as used by PeerTube) still work properly.

@opusforlife2
Copy link
Collaborator

Is it really alright to use an alpha version of a dependency? I remember F-Droid builds fail because alpha/beta versions of some Android build-related software aren't open source.

@wb9688
Copy link
Contributor Author

wb9688 commented Oct 14, 2020

@opusforlife2: Yes, the alpha versions of the AndroidX libraries are actually open-source (unlike the beta versions of the Android SDK itself), the following is the commits that were added in this alpha version compared to the latest non-alpha version: https://android.googlesource.com/platform/frameworks/support/+log/6880f4a443ed0d4e88e15c18044fbfc644c766e2..9f60cc700129e30cee9df020005c317fb39d32ec/room

@opusforlife2
Copy link
Collaborator

  • Import database from 0.20.0 👍
  • Import database from 0.19.8 👍
  • Subscription export 👍
  • Subscription import 👍
  • Markdown in descriptions 👍

@Redirion
Copy link
Member

Redirion commented Oct 15, 2020

@Redirion: Do you want to do that in some PR?

I am currently short of spare time :/ Maybe in 1-2 weeks.

@Stypox
Copy link
Member

Stypox commented Oct 16, 2020

@wb9688 could you also update Kotlin to 1.4.10?

@wb9688
Copy link
Contributor Author

wb9688 commented Oct 16, 2020

@Stypox: Already did it at https://github.com/TeamNewPipe/NewPipe/pull/4508/files#diff-49a96e7eea8a94af862798a45174e6ac43eb4f8b4bd40759b5da63ba31ec3ef7R4 ;)

However I have to rebase this PR because of disabling Ktlint for now :/

@Stypox Stypox linked an issue Oct 16, 2020 that may be closed by this pull request
@Stypox Stypox mentioned this pull request Oct 16, 2020
@Stypox
Copy link
Member

Stypox commented Oct 17, 2020

@Redirion upgrading ExoPlayer will hopefully fix this: in the middle of an mp4 stream the player suddenly says "Could not play this stream". From that point on trying to play the same cached stream will fail with the same error even if the playback is restarted. This error is given through logcat:

E/ExoPlayerImplInternal: Source error
      com.google.android.exoplayer2.ParserException: Invalid NAL length
        at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.readSample(Mp4Extractor.java:542)
        at com.google.android.exoplayer2.extractor.mp4.Mp4Extractor.read(Mp4Extractor.java:192)
        at com.google.android.exoplayer2.source.ProgressiveMediaPeriod$ExtractingLoadable.load(ProgressiveMediaPeriod.java:988)
        at com.google.android.exoplayer2.upstream.Loader$LoadTask.run(Loader.java:415)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1133)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:607)
        at java.lang.Thread.run(Thread.java:776)

By looking up on exoplayer's repo I found out this google/ExoPlayer#7698, which is probably fixed by google/ExoPlayer@f29af87, so we should be good

@opusforlife2
Copy link
Collaborator

@Stypox I think I asked at least one of those to test with WebM as well? I don't remember clearly.

@wb9688 wb9688 closed this Oct 24, 2020
@Stypox Stypox mentioned this pull request Nov 2, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App UI crash
4 participants