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

[media-library] Fix a crash when calling getAssetInfoAsync on slow motion videos on iOS #8802

Merged
merged 6 commits into from
Jun 23, 2020

Conversation

jarvisluong
Copy link
Contributor

Why and How

Described in: #8801

Test Plan

Choose to see details of a slow motion video on iOS Bare app, the app should not crash

Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

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

Overall it looks good 👍

@jarvisluong jarvisluong requested a review from lukmccall June 17, 2020 01:48
Copy link
Contributor

@lukmccall lukmccall left a comment

Choose a reason for hiding this comment

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

Thanks for applying all requested changes 🎉

@tsapeta tsapeta changed the title [expo-media-library] handle AVComposition case [media-library] Fix a crash when calling getAssetInfoAsync on slow motion videos on iOS Jun 23, 2020
@tsapeta tsapeta merged commit c466da0 into expo:master Jun 23, 2020
@jarvisluong jarvisluong deleted the bugfix/handle-slow-motion-video-ios branch June 23, 2020 12:33
@awinograd
Copy link
Contributor

Thanks for your fixes to expo-media-library @jarvisluong. They've been working perfectly for me.

Any chance we can publish a new version of the package, expo team? I'm currently manually applying this patch when building my app. On a general note, what's the normal cadence for publishing updates like this? Happy to wait until the normal time if there's a standard process.

Thanks all!

@tsapeta
Copy link
Member

tsapeta commented Jul 2, 2020

Yeah, sure, I've just published a new version — 8.3.0 🎉

We don't have any strict cadence for publishing updates other than just before SDK releases, however I'm trying to improve this. Before SDK38 we were publishing all packages at once and only during the release process, but now we're slowly pushing more module-centric approach forward to make our modules even more attractive than the alternative ones 😉 As expo modules often depend on others, we've had some concerns about publishing them more frequently, mostly due to compatibility reasons. However, thanks to some new tools that I've built recently, I hope that one day we will achieve our goal which is to publish them regularly (once per week?) by GitHub Actions — but until then, just let me know if you're waiting for the fix 😄

Unfortunately only bare workflow projects will benefit from this because we cannot release the Expo Client to the stores as often as we would like 😞

@awinograd
Copy link
Contributor

awinograd commented Jul 2, 2020

Thank you, @tsapeta! Much appreciated.

I look forward to all of the new tools being taken advantage of!

@mvjansen
Copy link

Hi @tsapeta, is there any way to get this bug fix working in an expo project (i.e. not a bare work flow project)? I have installed the 8.3.0 version of expo-media-library and created a new fresh ios build, but my app is still crashing on slow mo videos

@jarvisluong
Copy link
Contributor Author

Unfortunately only bare workflow projects will benefit from this because we cannot release the Expo Client to the stores as often as we would like 😞

Since this is a native change, you will need for a new expo client to include this fix

@mvjansen
Copy link

Hi @jarvisluong, I saw the comment for an updated expo client, but thought a build of a new .ipa file would incorporate the new native code and not have any dependence on the expo client when running as a standalone app. Obviously I'm missing something?

@jarvisluong
Copy link
Contributor Author

IMO the ipa build on Expo service should basically freeze the version of the dependencies to make sure the build is stable. I think it's best that you eject to the bare workflow and then be able to build the new native version with the fix. I believe the eject process is not as painful as it used to be (in fact I use bare workflow for all projects now)

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.

5 participants