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

libstagefright.so SIGSEGV on various Android 7/7.1 devices after upgrading to 2.5.1 #3215

Closed
dbrain opened this issue Aug 29, 2017 · 11 comments

Comments

@dbrain
Copy link
Contributor

dbrain commented Aug 29, 2017

Issue description

After upgrading ExoPlayer to 2.5.1 (from 2.4.4) we are seeing a lot of user reports of a libstagefright.so SIGSEGV. Playback starts, but a crash occurs when the player is released. This may be specific to Widevine+DASH, as the demo app will not crash using the Youtube DASH demos, while it crashes the same as our application on Widevine+DASH demos.

I have a feeling this may be related to the recent CENC support added:
#1989
But I also don't know what I'm talking about, so who knows ;)

I had a quick play at making DefaultDrmSessionManager return false for canAquireSession if the type was cenc etc. (that whole conditional), but ExoPlayer never seemed to call that method at all. Will fiddle a bit more, but thought I'd raise this ASAP.

A remote colleague has a device that can reproduce the crash (Motorola G5 Plus), but there's a bit of a lag getting debug information. Sorry if this contains limited information.

Reproduction steps

  1. Launch the demo app on an effected device
  2. Start a Widevine+DASH demo (WV: Secure SD & HD (cenc,MP4,H264) definitely reproduces this)
  3. Let playback begin
  4. Press back leaving playback, releasing the player
  5. Crash should occur

Link to test content

Reproducible in the demo WV: Secure SD & HD (cenc,MP4,H264), so not required.

Version of ExoPlayer being used

2.5.1

Device(s) and version(s) of Android being used

Seems to be 7.0/7.1 specific, but multiple devices. Easily reproducible, as it occurs from what I can tell any time when releasing the player after playback has begun.

Here are the play store device/version graphics:
image

A full bug report captured from the device

Here's a zipped adb bugreport > libstagefright-widevine-dash-crash.log from our application (iflix.play.dbg).
libstagefright-widevine-dash-crash.zip

@ojw28
Copy link
Contributor

ojw28 commented Aug 29, 2017

How are you setting the Surface on the player, and is it possible that the Surface is being cleared before the player is released? Note that if you're passing the SurfaceView, TextureView or SurfaceHolder to the player, this may be happening internally.

If so then I'd suggest DummySurface is the most likely cause. To verify this suggestion, could you please try locally modifying DummySurface.isSecureSupported to return false, and then see if that makes the issue go away on an affected device? Thanks!

@dbrain
Copy link
Contributor Author

dbrain commented Aug 29, 2017

@ojw28 The app is using SimpleExoPlayerView and SimpleExoPlayer and leaving it up to SimpleExoPlayerView to handle the Surface side of things. There's nothing specific in our codebase touching the Surface, and SimpleExoPlayer.release() is called onStop().

Which is pretty much exactly how the demo app is set up looking at it, and is also crashing at the same point.

I'll get a build of the demo app with the DummySurface change to my colleague with a device that can reproduce the issue and let you know. Thanks for the quick response!

@dbrain
Copy link
Contributor Author

dbrain commented Aug 29, 2017

@ojw28 Looks like that change resolved the crash. How do we go about getting a fix in for a future release?
Happy to create a PR myself if you can give me some guidance on the best solution.

Seems there's a deviceNeedsSecureDummySurfaceWorkaround method that returns true for specific devices, but there's over 20 affected devices that I know about in this scenario.

For now we'll be dropping the application back to 2.4.4.

@ojw28 ojw28 added the bug label Aug 29, 2017
@ojw28
Copy link
Contributor

ojw28 commented Aug 29, 2017

That's unfortunate. I think we'll need to restrict the feature and roll it out more cautiously, due to the number of devices that are affected. I suspect all (or nearly all) of the affected devices return false from packageManager.hasSystemFeature(PackageManager.FEATURE_VR_MODE_HIGH_PERFORMANCE);, so we could try requiring that to be true if SDK_INT < 26 for use of secure DummySurface to be enabled.

@dbrain
Copy link
Contributor Author

dbrain commented Aug 31, 2017

Does (Util.SDK_INT >= 24 && Util.SDK_INT < 26 && !hasVrModeHighPerformanceSystemFeatureV24(context.getPackageManager())); look sane enough?

I brought back the hasVrModeHighPerformanceSystemFeatureV24 method which seems to have been removed in dev-v2. So I'm checking that device is >= 24 i.e. has FEATURE_VR_MODE_HIGH_PERFORMANCE at all, < 26 because presumably the issue is fixed after 25 and that it does not have the FEATURE_VR_MODE_HIGH_PERFORMANCE system feature.

Just verifying it's happy on the one effected device I have "available" to me.

@dbrain
Copy link
Contributor Author

dbrain commented Aug 31, 2017

The above solved the issue on the Moto G5 Plus at least.

ojw28 added a commit that referenced this issue Sep 4, 2017
…face_exclusions

#3215 Additional secure DummySurface device exclusions
@ojw28 ojw28 closed this as completed Sep 4, 2017
ojw28 added a commit that referenced this issue Sep 5, 2017
Issue: #3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167505797
ojw28 added a commit that referenced this issue Sep 6, 2017
Issue: #3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167505797
ojw28 added a commit that referenced this issue Sep 9, 2017
Issue: #3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167505797
hishamMuneer added a commit to hishamMuneer/ExoPlayer that referenced this issue Sep 14, 2017
* Minimal change to expose segment indices in DefaultDashChunkSource

Issue: google#3037

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164614478

* Clean up terminology for MediaSession extension

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164705750

* Fix minor Javadoc error

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164706078

* Document usage of the RTMP extension

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164706135

* set mediaSession flags properly and keep queue in sync when timeline changes

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164774783

* Support multiple video/text/metadata outputs

We've seen more than one issue filed where a developer has
registered a video listener and been confused by the fact
their SimpleExoPlayerView no longer works properly. There
are also valid use cases for having multiple metadata/text
outputs.

Issue: google#2933
Issue: google#2800
Issue: google#2286
Issue: google#2240

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164839882

* Fix maskingX variables when timeline becomes empty

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164840037

* Restrict usage of secure DummySurface for all Samsung devices.

* Support H262 video in MP4

* Fix possible subrip timing line NPE

* Support crop mode for AspectRatioFrameLayout

* Add flag to CachedContentIndex to disable encryption.  This allows the encryption feature
to be disabled gracefully: encrypted index files may be read, but plaintext will be written.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165196508

* Disable secure dummy surface on all Samsung N devices

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165291627

* Update instructions to include Google Maven repository

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165291982

* Destroy GL context when releasing dummy surface

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165293386

* Restore the interrupted flag after blocking operations

If the main thread was interrupted during
ExoPlayerImplInternal.blockingSendMessage/release, the interrupted flag was
immediately set but then wait() was called on the next iteration. wait() would
immediately throw InterruptedException, causing the main thread to spin until
the blocking operation completed.

Instead of resetting the flag immediately, reset it after the blocking
operation completes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165426493

* Work around issue with Xiaomi JB devices

Issue: google#3171

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165577562

* Improve FORMAT_UNSUPPORTED_DRM related documentation and logging

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165580016

* Improve MediaSource/MediaPeriod documentation

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165628229

* expose setPropertyByteArray, setPropertyString

export setPropertyByteArray, setPropertyString of DefaultDrmSessionManager for easy customization.

* Minor style tweaks

* Allow the app to specify extra ad markers

Issue: google#3184

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165895259

* Remove isFirstWindow/isLastWindow from Timeline.

These methods are only used in one place, and offer duplicate
functionality to checking getNext(Previous)WindowIndex == C.INDEX_UNSET.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165910258

* Fix broken link + minor doc tweak

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165920927

* Handle size==0 in MP4 atoms

Issue: google#3191

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165925148

* Use flavorDimensions for external demo app

- This is soon becoming mandatory.
- It also looks like future versions of com.android.tools.build
  are being distributed via Google's Maven repository.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166058299

* Support aspect ratio fill mode for AspectRatioFrameLayout

* Support zoom mode for AspectRatioFrameLayout

* Allow subclasses to customize the MediaFormat

Make getMediaFormat protected so that subclasses can set additional MediaFormat
keys. For example, if the decoder output needs to be read back via an
ImageReader as YUV data it is necessary to set KEY_COLOR_FORMAT to
COLOR_FormatYUV420Flexible.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166195211

* Use Math.abs in Sonic.java

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=166820970

* Add support for new codecs parameter string

* google#3215 Additional secure DummySurface device exclusions

* Use UTF-8 everywhere

UTF-8 is the default charset on Android so this should be a no-op change, but
makes the code portable (in case it runs on another platform).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167011583

* Fix ContentDataSource bytesRemaining calculation

The bytesRemaining didn't always take into account any skipped bytes, which
meant that reaching the end of the file was not correctly detected in read().

Issue: google#3216

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167016672

* Allow more aggressive switching for HLS with independent segments

We currently switch without downloading overlapping segments, but
we do not actually switch more aggressively. This change fixes
this. Note there's an implicit assumption made that if one media
playlist declares independent segments, the others will too. This
is almost certainly true in practice, and if it's not the penalty
isn't too bad (the player may try and switch to a higher quality
variant one segment's worth of buffer too soon).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167120992

* HDR 10 bits: Use a separate sampler for U and V dithering.
Using the same sampler introduced some minor horizontal scratches.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167347995

* Fix typo

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167474040

* Minor cleanup to AspectRatioFrameLayout

* Update moe eqiuvalence

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167488837

* Allow EXIF tracks to be exposed

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167493800

* Adding missing license header in IMA build.gradle

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167496569

* Additional secure DummySurface device exclusions

Merge: google#3225

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167502127

* Be robust against unexpected EOS in WebvttCueParser

Issue: google#3228

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167504122

* Rewrite logic for enabling secure DummySurface

Issue: google#3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167505797

* Upgrade gradle plugin / wrapper

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167579719

* Don't use MediaCodec.setOutputSurface on Nexus 7 with qcom decoder

Issue: google#3236

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167581198

* Fix position reporting during ads when period has non-zero window offset.

Reporting incorrect positions for ad playbacks was causing IMA to
think the ad wasn't playing, when in fact it was.

Issue: google#3180

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167702032

* Workaround for SurfaceView not being hidden properly

This appears to be fixed in Oreo, but given how harmless
the workaround is we can probably just apply it on all
API levels to be sure.

Issue: google#3160

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167709070

* DecryptionException cleanup + add missing header

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167711928

* Pick up rtmpClient fix

Issue: google#3156

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167718081

* Enable rtmp in external demo app with extensions

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168007345

* Destroy EGLSurface during DummySurface cleanup

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168020525

* Fix attr inheritance in SimpleExoPlayerView

When creating PlaybackControlView inside SimpleExoPlayerView,
we want certain attributes to be passed through. This lets you
set control attributes on the SimpleExoPlayerView directly. We
don't want all attributes to be propagated though; only our own
custom ones.

Not sure if there's a cleaner way to do this. Pragmatically this
solution seems ... ok :)?

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167619801

* Add possibility of forcing a specific license URL in HttpMediaDrmCallback

Resubmit of google#3136

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=164971900

* Fix build for release

* Bump to 2.5.2

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=168155713

* Update extension README with usage instructions

Issue: google#3162

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165572088

* Tweak and add READMEs + remove refs to V1

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=165578518

* Update README.md
@jpardogo
Copy link

jpardogo commented Sep 29, 2017

Moto G5 Plus 7.0 have been fixed in 2.5.2 but it is still happening in Moto Z 7.1.1 (the only other device that I could test in 2.5.1). Never happened in 2.4.4 and below (version I am still using for this reason). I think this issue should be reopen.

@ojw28
Copy link
Contributor

ojw28 commented Oct 2, 2017

Please provide a full bug report captured just after encountering the issue. Thanks.

@dbrain
Copy link
Contributor Author

dbrain commented Nov 14, 2017

Now that we've upgraded to 2.5.4 I'm also seeing this issue on Motorola Moto Z (griffin), 3072MB RAM, Android 7.1 from Google Play Console reports. Unable to retrieve a bug report as I do not have access to this device. @jpardogo could you produce this?

Currently doing a staged rollout, so it may be occurring on other devices as well. Original device that could reproduce the crash is fine (Moto G5 Plus).

@ojw28
Copy link
Contributor

ojw28 commented Dec 6, 2017

Could you provide a breakdown of the (device,api-level) combinations on which you're still seeing this type of failure?

@dbrain
Copy link
Contributor Author

dbrain commented Dec 6, 2017

@ojw28 I'm struggling to get useful device stats specifically for the version running 2.5.4. We use Crashlytics and it doesn't seem to pick up the native crashes, so all I have to go by is the developer console which doesn't seem to filter on app version for a specific crash.

Skimming reports seems to imply it's still limited to Motorola Moto Z (griffin), 3072MB RAM, Android 7.1, seeing around 300 reports a week with around 300k avg daily active users. Looks like only 165 unique users though so it's not epic, the users hitting this issue get a crash every time they leave the player though.

ojw28 added a commit that referenced this issue Dec 12, 2017
Issue: #3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178218535
ojw28 added a commit that referenced this issue Dec 12, 2017
Issue: #3215

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=178218535
@google google locked and limited conversation to collaborators Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants