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

speed optimization on convert(Frame):Bitmap #379

Merged
merged 1 commit into from Apr 25, 2016
Merged

Conversation

pfn
Copy link
Contributor

@pfn pfn commented Apr 7, 2016

eliminate individual get/put calls as much as possible

@saudet
Copy link
Member

saudet commented Apr 8, 2016

Cool, are you sure this is faster though? AFAIK, Android isn't very efficient even on bulk operations. In any case, let's also cache inplus, in a manner similar to buffer to avoid reallocating memory all the time! And I'll merge this is. Thanks

@pfn
Copy link
Contributor Author

pfn commented Apr 8, 2016

Oops, I forgot to add the inplus member when I was refactoring, but it is cached and created together with buffer.

I'll fix that Monday.

As for performance, I've been testing and profiling on N preview and it is dramatically faster. Calling convert during live camera preview frames drops cpu time from 60% to something like 20%.

Behavior prior to N preview might be a little different, I haven't tested.

@saudet
Copy link
Member

saudet commented Apr 8, 2016

Faster then, cool!

BTW, a better way of doing it without inplus is to copy directly from the original buffer inside the loop everything expect the very last pixel ;)

@pfn pfn force-pushed the master branch 2 times, most recently from abdeb44 to 612ad1e Compare April 8, 2016 16:08
@pfn
Copy link
Contributor Author

pfn commented Apr 8, 2016

I forgot today is Friday and not Saturday. So, my changes are updated per feedback.

eliminate individual get/put calls as much as possible
int b = in.get(y * stride + 3 * x + 2) & 0xff;
rgba = (r << 24) | (g << 16) | (b << 8);
}
buffer.putInt(y * rowBytes + 4 * x, (rgba << 8) | 0xff);
Copy link
Member

Choose a reason for hiding this comment

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

There's going to be a problem here I think for the last pixel. It's not being shifted.

@saudet
Copy link
Member

saudet commented Apr 9, 2016

Great, thanks! A couple of places to fix as noted above, and it's good to merge.

@saudet
Copy link
Member

saudet commented Apr 9, 2016

But looking at this more closely, the order doesn't seem to be right. We assume an input of BGR. To be less confusing, we should inverse the order of "r" and "b". And then to make sure it always works as expected, we should force in to be LITTLE_ENDIAN. With buffer as BIG_ENDIAN it should work properly.

@saudet saudet merged commit bc2ddec into bytedeco:master Apr 25, 2016
@pfn
Copy link
Contributor Author

pfn commented Apr 25, 2016

I haven't had a chance to get back to this to improve it further. I will
submit another pr when I get a chance

@saudet
Copy link
Member

saudet commented Apr 25, 2016

No problem, I figured I should add a test and move the old code there, to compare the results between new candidate code, against known good results. And while I'm at it I fixed your code to make the test pass. :) Thanks for the contribution and yes feel free to optimize further!

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

2 participants