Created attachment 318062 [details] [review]
Optimize buffers and remove extra memcpy()

For jpeg files with uneven dimensions or grayscale, a different functions are used which is using the memcpy() and extra buffers.

Review of attachment 318062 [details] [review]:

A comment that show you have researched why it was done like this before and what you are doing to avoid the issue that the copies where fixing seems rather important. You seem to simply use pointers directly and no code is adding any memory management. This is in general suspicious, I didn't look in details, but please, demonstrate that you have done some research about why it was like this, and demonstrate that you keep proper memory management without those copies.

The jpeg_read_raw_data() function needs an array 3 pointers, each further constituting an array of 16 gpointer's which further point to actual data. By convention, a local copy of buffers was made to pass to jpeg_read_raw_data(). 

But while optimizing the jpegdec for even dimensional pictures, these memcpy() kind of overheads were removed from gst_jpeg_dec_decode_direct(). As gst_jpeg_dec_decode_indirect() was meant to be used for odd dimensional pictures, it may have been overlooked for optimization at that moment.

As for the point of usage, these array of gpointer's are used only to read actual data and hence a local copy is not necessary. Also, there is no parallel decoding going on, so underlying pointers are not meant to change, so local copy is unnecessary.

Nicolas, does this look good for you now?

I would prefer give this patch some stress test with queues and specialized buffers pools before landing. I do hope this patch is fine, but there is no test to prove it. The commit comment can still be improved.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/249.