Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[Video_Player] Remove the deprecated API reference. #3669

Merged
merged 3 commits into from
Mar 8, 2021
Merged

[Video_Player] Remove the deprecated API reference. #3669

merged 3 commits into from
Mar 8, 2021

Conversation

zhenqiu1101
Copy link
Contributor

@zhenqiu1101 zhenqiu1101 commented Mar 3, 2021 •
edited
Loading

exoPlayer.setAudioStreamType has been deprecated. Remove it from the video_player plugin.

Doc: https://developer.android.com/reference/android/media/MediaPlayer#setAudioStreamType(int)

Pre-launch Checklist

  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla google-cla bot added the cla: yes label Mar 3, 2021
@zhenqiu1101 zhenqiu1101 requested a review from cyanglaz March 3, 2021 04:24
@stuartmorgan-g
Copy link
Contributor

It's deprecated in API 21, but the plugin supports building back to 16, so this will regress running on old versions. Can you explain the problem you are trying to solve? Are you using alternate build files with a newer target?

@zhenqiu1101
Copy link
Contributor Author

This API has been removed entirely from the internal exoplayer library. So the internal plugin is diverged from the upstream one. But I'm not sure if it's safe to remove this reference in the upstream as you mentioned. I've talked with Chris. Chris suggest me to create a PR to get some feedback. We can handle this difference for now if this causes regression.

@cyanglaz
Copy link
Contributor

cyanglaz commented Mar 3, 2021 •
edited
Loading

Is it possible to catch NoSuchMethodException? Or does Java has something similar to respondToSelector:

@stuartmorgan-g
Copy link
Contributor

Oh I see, I missed that this is ExoPlayer rather than Media player. So I think the question becomes, does the newer ExoPlayer API work down-level?

@zhenqiu1101
Copy link
Contributor Author

I think so. Actually, the exoPlayer.setAudioStreamType API has been removed from the source. But I'm not familiar with the upstream dependencies. I may look at the wrong repo. https://github.com/google/ExoPlayer/blob/release-v2/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java

@zhenqiu1101
Copy link
Contributor Author

Kindly ping.

@stuartmorgan-g
Copy link
Contributor

I think so.

Can you link to documentation/source/discussion that supports this?

Actually, the exoPlayer.setAudioStreamType API has been removed from the source.

Is that because the new method works downlevel so isn't needed, or has the current version dropped support for older versions of Android?

We need definitive confirmation that this isn't going to break the plugin for Android versions we still support before proceeding.

@zhenqiu1101
Copy link
Contributor Author

See here: https://github.com/google/ExoPlayer/blob/r2.9.6/library/core/src/main/java/com/google/android/exoplayer2/SimpleExoPlayer.java#L499

setAudioStreamType internally use setAudioAttributes(AudioAttributes) which is also deprecated later and should use setAudioAttributes(AudioAttributes, boolean) instead.

stuartmorgan-g
stuartmorgan-g approved these changes Mar 8, 2021
View reviewed changes
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, makes sense then! LGTM

@stuartmorgan-g stuartmorgan-g added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Mar 8, 2021
@fluttergithubbot fluttergithubbot merged commit 728129a into flutter:master Mar 8, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 8, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 8, 2021
@engine-flutter-autoroll
728129a [Video_Player] Remove the deprecated API reference. (flutter/…
18ca231 
…plugins#3669)
fluttergithubbot pushed a commit to flutter/flutter that referenced this pull request Mar 9, 2021
@engine-flutter-autoroll
728129a [Video_Player] Remove the deprecated API reference. (flutter/…
6e11fd9 
…plugins#3669) (#77649)
yasargil added a commit to yasargil/plugins that referenced this pull request Mar 18, 2021
@yasargil
Merge branch 'master' into gmain
f6f7fdd 
* master: (49 commits)
  Prep for alignment with Flutter analysis options (flutter#3703)
  [google_maps_flutter_platform_interface] Mark constructors as const for ids (flutter#3718)
  [image_picker] Endorse image_picker_for_web (flutter#3717)
  Add missing licenses, and add a check (flutter#3720)
  [ci] Add libgcrypt to Docker image. (flutter#3711)
  Reorder the checkboxes in the PR template (flutter#3666)
  Re-endorse connectivity_for_web (flutter#3708)
  Fix missing declaration of windows' default_package (flutter#3705)
  Typos in comments (flutter#2846)
  Skip flutter upgrade for pod linting Cirrus task (flutter#3700)
  [cross_file] Delete. (flutter#3698)
  [tool] Improve check version ci so that it enforces the version in CHANGELOG and pubspec matches. (flutter#3678)
  Streamline CI setup, and reenable macOS credits (flutter#3697)
  [video_player] fixed misleading size and aspect ratio documentation (flutter#3668)
  [image_picker] Implemented 2860 and added Unit Test to test functionality (flutter#3685)
  [shared_preferences] Fix concurrent modification of the shared preferences on Android (flutter#3684)
  [extension_google_sign_in_as_googleapis_auth] Deleted. (flutter#3694)
  Skip pod lint tests (flutter#3692)
  [Video_Player] Remove the deprecated API reference. (flutter#3669)
  [google_sign_in] fix test(flutter#3690)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stuartmorgan-g stuartmorgan-g stuartmorgan-g approved these changes

@cyanglaz cyanglaz Awaiting requested review from cyanglaz

Assignees
No one assigned
Labels
cla: yes p: video_player platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@zhenqiu1101 @stuartmorgan-g @cyanglaz @fluttergithubbot