GNOME Bugzilla – Bug 583047
[jpegdec] optimise buffer handling when parsing frames
Last modified: 2010-07-19 18:31:19 UTC
The buffer handling seems to be a bit suboptimal when incoming data is parsed into frames (frequent merging/copying etc.). This is particularly noticable when doing things like filesrc ! jpegdec ! ... for large pictures where data will be deliverered in 4kB chunks.
I'll try to look at this while making the jpegparse element. I'll submit both as separate patches. (Can someone with sufficient power add 583098 as a dependency of this bug?)
Created attachment 135881 [details] [review] Don't rescan from the start for every buffer pushed in I created a basic jpegparse element (see attachment (id=135877) to Bug 583098 ). This patch applies on top of that. It's not entirely trivial to port this patch to jpegdec itself, as I'm using an adapter rather than a custom buffer. Note that there still is a copy for every buffer pushed in. To avoid this, something should be added to GstAdapter so you can peek at an offset.
> Note that there still is a copy for every buffer pushed in. To avoid this, > something should be added to GstAdapter so you can peek at an offset. gst_adapter_copy()and gst_adapter_masked_scan_uint32() might come in handy here. If that's not really what you're after, please file a feature request bug against the core so we can add the kind of function you need.
Created attachment 159883 [details] [review] Improve buffer scanning Earlier patch seems to be outdated (a.o. no longer applies to current jpegparse). Current version also partially retains last position. This patch adds some more position retaining that should then cover functionality of previous patch.
Created attachment 159884 [details] [review] jpegdec: optimise buffer scanning Similar patch for jpegdec's parsing.
While above patches optimise buffer (re)scanning, they do not so much address buffer handling. gst_adapter_masked_scan_uint32() might help to look for a 0xFF 0xYY pattern, but would help more if e.g. it (or alike) would also return the guint32 that actually matched, so the 0xYY could be used to decide how to proceed. Also, performing a lot of _scan_uint32 at a high offset in an adapter with lots of (small?) buffers means that each scans has to do a linear search first for the right buffer for that offset. This overhead is probably less than that of copying buffers, but maybe some state for progressive scan might be more optimal ??
In -good: commit 52c71352e034726955ac285daf132b43bd9fb36c Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu Apr 29 16:26:49 2010 +0200 jpegdec: optimise buffer scanning Specifically, when needing more data, do not rescan from start next time around, but resume from last position. See also #583047. In -bad: commit d68a4c1dcc6da81bc4aa68574095b9f6aae93247 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu Apr 29 16:33:44 2010 +0200 jpegparse: improve buffer scanning Specifically, when scanning for entropy data segment length and needing more data, do not rescan from start next time around, but resume at last position. See also #583047.
Created attachment 162102 [details] [review] Optimize buffer handling when parsing This patch introduces an adapter in jpegdec, and uses a.o. adapter scanning for subsequent parsing [*]. In particular, repeated buffer copying is avoided, though one copy is likely (and unavoidably?) still present at _take time. [*] Note that some enhanced adapter API from bug #619828 is used (which, if so desired/preferred, could probably be avoided.)
(In reply to comment #8) > though one copy is likely (and unavoidably?) still present at _take time. Not unavoidable. libjpeg is optimized for scatter-gather operation. It processes all the available data, and calls fill_input_buffer when it needs more data. I'm not entirely sure what the interface is, but I think you can just update jsrc.pub.next_input_byte and jsrc.pub.bytes_in_buffer from within that function. If you use gst_adapter_available_fast(), you can make sure the _peek never does any copying.
Created attachment 162374 [details] [review] Optimize buffer handling when parsing Essentially as before, but remove a minor debug leftover.
Created attachment 162375 [details] [review] Use libjpeg scatter-gather operation to avoid data copying As suggested, use some libjpeg API to allow for scatter-gather and avoid data copying that way.
Created attachment 162439 [details] [review] jpegparse: optimize image parsing using adapter API This patch pretty much backports latest image parsing in jpegdec (using adapter) to jpegparse.
In -good: commit a391bf52cc3c580c7a0a2316ca52eb66da3b85c1 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon May 31 12:45:01 2010 +0200 jpegdec: use libjpeg scatter-gather operation to avoid data copying Fixes #583047 (more). commit 58fbcf01e5d4b175a7c012dbf37c9688d3bfffdc Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu May 27 15:45:23 2010 +0200 jpegdec: optimize buffer handling when parsing Use an adapter to collect incoming data, and use adapter API to scan and peek. Fixes #583047. In -bad: commit e6cf05b1141c6728c7e28d3890d1070fc8bf712f Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon May 31 18:30:19 2010 +0200 jpegparse: optimize image parsing Use adapter API for scanning and peeking to reduce buffer copying. See #583047.
FWIW, this caused a regression and it was now fixed by commit da1c816358b947c3a4941bef76882179e2330332 in -good.