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

Make high-profile available #73

Merged
merged 3 commits into from Aug 19, 2019
Merged

Make high-profile available #73

merged 3 commits into from Aug 19, 2019

Conversation

shts
Copy link
Contributor

@shts shts commented Feb 13, 2019

No description provided.

@ffMathy
Copy link

ffMathy commented Mar 12, 2019

@ypresto this fixes #72 which also leads to various issues with Cordova's video editor plugin, which is using your plugin and has this error (for instance, on OnePlus devices).

@maxpaj
Copy link

maxpaj commented Mar 20, 2019

Can this be merged?

@natario1
Copy link

After trying with #71 some months ago I decided to go forward and make a fork which I am maintaining and has dozens of commits ahead. I have removed this profile limitation a long time ago.

If you want, come take a look at https://github.com/natario1/Transcoder . Docs are still missing, but the demo app will show the new API surface.

@ypresto
Copy link
Owner

ypresto commented Mar 23, 2019

Sorry for late response.
I have very very little time for coding outside of work because my 👶 is 🏃 😊 💥 💥 👫 😱 😭 😫

As supported video format list, high profile encoder is not supported yet. This means there is no guarantee that codecs of all the devices will output valid video data.

This library highly depends on hardware codec impl, and sometimes nasty bugs were produced like #8 and some Nexus-4-era glitch issues.
For the other hand high profile w/ 1080p might produce glitch even if 720p does not.
Important thing is that not all the devices use Snapdragon.

So I am conservative for profile and resolution support. This is also because this lib provides transcoding for the family album (yes, it is very precious memory which should never get glitch) app mitene, which has over a million Android users.

But maybe you can check compatibility like this, or by xml file stored in /etc (?) of device. Or by testing running codec before using it.

(Ah, I remembered why I added this restriction. libstagefright code in older Android forcefully overwrites profile before passing it to codec, perhaps.)

@coreysherman
Copy link

coreysherman commented Apr 3, 2019

@ypresto I'm not quite sure I understand the response with regards to merging this branch. As mentioned above, your plugin is used by Cordova's video editor plugin, which is also used by ionic native. Are there any plan to merge this fix in?

@ypresto
Copy link
Owner

ypresto commented Apr 5, 2019

I was misunderstanding that cordova video plugin returns null from format strategy (so original AVC video data is used regardless of its profile), but actually it does not.

So, in OnePlus 6 (I also have it) era device, the codec generates high-profile video even without passing any profile parameter. Is it right?

Profile validator was introduced because lollipop-era device does not well support non-baseline video.
See this nasty XXX line in libstagefright of lollipop, it overwrites profile value to baseline without throwing error 😂 ↓
https://android.googlesource.com/platform/frameworks/av/+/lollipop-release/media/libstagefright/OMXCodec.cpp#1226

My recommendation at lollipop-era was stick with baseline (except for decoder), but now we can just remove the profile validator, and would be better to check the profile is supported by CodecProfileLevel or etc.

@@ -36,7 +37,8 @@ public static void validateVideoOutputFormat(MediaFormat format) {
}
ByteBuffer spsBuffer = AvcCsdUtils.getSpsBuffer(format);
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove lines below here, as profile validation is already not necessary.
#73 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pengkobe
Copy link

Any update? i am waiting so long time...

@martinbertinat
Copy link

@pengkobe I'm waiting too.. But.. Can we help in any wey @ypresto ?

@todoscontrajuan
Copy link

It looks like this can be merge, @ypresto can you approve this?

@Maxgamerboy1
Copy link

Bump! I would also like to consume this change. Thanks!

@ypresto ypresto merged commit c0ee729 into ypresto:master Aug 19, 2019
@davidTaveras48
Copy link

@ypresto im developing on ionic and im having this error :
Non-baseline AVC video profile is not supported by Android OS, actual profile_idc: 100

just on android 9.x or higher.

any solution for this ?

thanks

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.

None yet