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]
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]
As before, but now another variant (masked_scan_uint32_flush) is added as well, as suggested previously.
Comment on attachment 163559 [details] [review]
>+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.
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.
> 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.
Author: Mark Nauwelaerts <firstname.lastname@example.org>
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.