GNOME Bugzilla – Bug 619828
[API] adapter: add masked_scan_uint32_peek
Last modified: 2010-06-14 13:14:43 UTC
Created attachment 162100 [details] [review] Add masked_scan_uint32_peek That is, extend masked_scan_uint32 to not only returned the offset where a match was found (if any), but also the uint32 that matched at that location, as it is available for free anyway. It is fairly typical to scan for some marker/pattern and then need some other byte(s) that are pretty much nearby, which can then be obtained immediately this way, without having to go through _copy.
Created attachment 162101 [details] [review] Optimize adapter progressive scan and copy Though this patch is technically independent of API in question, it also addresses a typical progressive scanning use case. In such case, it is advantageous to retain last "poked" position and to base subsequent scan/copy on that. Btw, see e.g. bug #583047 for such a use-case example.
Any comments, naming suggestions or otherwise ?
+ guint gst_adapter_masked_scan_uint32_peek (GstAdapter * adapter, + guint32 mask, guint32 pattern, + guint offset, guint size, + guint32 * value); Why not, seems like a worthwhile addition to me - if you have a use for it, go for it. Some additional / alternative ideas: - maybe add masked scan variants that automatically flush all data to the match? Then you could just return a boolean instead of an offset, and/or return the peek value directly, using 0 for 'not found' (with guards to make sure that mask and pattern won't yield 0)
Created attachment 163559 [details] [review] Add masked_scan_uint32_peek As before, but now another variant (masked_scan_uint32_flush) is added as well, as suggested previously.
Comment on attachment 163559 [details] [review] Add masked_scan_uint32_peek >+guint >+gst_adapter_masked_scan_uint32_flush (GstAdapter * adapter, guint32 mask, >+ guint32 pattern, guint offset, guint size) >+{ >... >+ location = gst_adapter_masked_scan_uint32_peek (adapter, mask, pattern, >+ offset, size, &result); >+ if (location >= 0) >+ gst_adapter_flush (adapter, location); >+ >+ return result; >+} I wonder if we should still flush SIZE-N byte here in the 'not found' case.
wonder++ Slight problem here is that there no use-case (yet) to evaluate, and that this particular one (however it flushes) can easily be implemented "locally" (when needed) as it only combines other adapter API, whereas the _peek one needs adapter API and implementation patching.
1) I think we should leave the flush version out until someone actually needs it. 2) I'd use a different name anyway, though I'm not sure what name yet.
> wonder++ > > Slight problem here is that there no use-case (yet) to evaluate, and that this > particular one (however it flushes) can easily be implemented "locally" (when > needed) as it only combines other adapter API, whereas the _peek one needs > adapter API and implementation patching. Let's just skip the other one then for now. I thought this kind of usage was what you were after, but I guess I didn't look closely enough at the code.
commit d5ed339f209cd8492e2d7af569603afa33f67cb4 Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Thu May 27 12:15:22 2010 +0200 adapter: add extended masked_scan_uint32_peek that also provides matching value Also add to .def and docs. Fixes #619828. API: gst_adapter_masked_scan_uint32_peek