After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 583047 - [jpegdec] optimise buffer handling when parsing frames
[jpegdec] optimise buffer handling when parsing frames
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal enhancement
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 583098
Blocks:
 
 
Reported: 2009-05-18 14:05 UTC by Tim-Philipp Müller
Modified: 2010-07-19 18:31 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Don't rescan from the start for every buffer pushed in (10.79 KB, patch)
2009-06-03 14:14 UTC, Arnout Vandecappelle
none Details | Review
Improve buffer scanning (2.82 KB, patch)
2010-04-29 14:34 UTC, Mark Nauwelaerts
committed Details | Review
jpegdec: optimise buffer scanning (4.99 KB, patch)
2010-04-29 14:35 UTC, Mark Nauwelaerts
committed Details | Review
Optimize buffer handling when parsing (14.71 KB, patch)
2010-05-27 13:58 UTC, Mark Nauwelaerts
none Details | Review
Optimize buffer handling when parsing (14.67 KB, patch)
2010-05-31 12:20 UTC, Mark Nauwelaerts
committed Details | Review
Use libjpeg scatter-gather operation to avoid data copying (3.57 KB, patch)
2010-05-31 12:26 UTC, Mark Nauwelaerts
committed Details | Review
jpegparse: optimize image parsing using adapter API (10.50 KB, patch)
2010-06-01 08:58 UTC, Mark Nauwelaerts
committed Details | Review

Description Tim-Philipp Müller 2009-05-18 14:05: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.
Comment 1 Arnout Vandecappelle 2009-05-18 16:20:42 UTC
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?)
Comment 2 Arnout Vandecappelle 2009-06-03 14:14:33 UTC
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.
Comment 3 Tim-Philipp Müller 2009-06-12 12:40:06 UTC
> 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.
Comment 4 Mark Nauwelaerts 2010-04-29 14:34:08 UTC
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.
Comment 5 Mark Nauwelaerts 2010-04-29 14:35:23 UTC
Created attachment 159884 [details] [review]
jpegdec: optimise buffer scanning

Similar patch for jpegdec's parsing.
Comment 6 Mark Nauwelaerts 2010-04-29 15:12:32 UTC
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 ??
Comment 7 Mark Nauwelaerts 2010-04-30 17:55:40 UTC
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.
Comment 8 Mark Nauwelaerts 2010-05-27 13:58:37 UTC
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.)
Comment 9 Arnout Vandecappelle 2010-05-27 14:35:47 UTC
(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.
Comment 10 Mark Nauwelaerts 2010-05-31 12:20:03 UTC
Created attachment 162374 [details] [review]
Optimize buffer handling when parsing

Essentially as before, but remove a minor debug leftover.
Comment 11 Mark Nauwelaerts 2010-05-31 12:26:14 UTC
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.
Comment 12 Mark Nauwelaerts 2010-06-01 08:58:22 UTC
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.
Comment 13 Mark Nauwelaerts 2010-06-14 13:52:06 UTC
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.
Comment 14 Thiago Sousa Santos 2010-07-19 18:31:19 UTC
FWIW, this caused a regression and it was now fixed by commit da1c816358b947c3a4941bef76882179e2330332 in -good.