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 587561 - gst_byte_reader_masked_scan_uint32 improvement
gst_byte_reader_masked_scan_uint32 improvement
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal blocker
: NONE
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-07-01 21:07 UTC by Carl-Anton Ingmarsson
Modified: 2009-07-16 13:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch with the above mentioned changes (1.03 KB, patch)
2009-07-01 21:09 UTC, Carl-Anton Ingmarsson
none Details | Review

Description Carl-Anton Ingmarsson 2009-07-01 21:07:55 UTC
I think it makes sense if gst_byte_reader_masked_scan_uint32 always returns the offset from the beginning of the data. I've attached a patch that does this. I've also tried to clarify the documentation for the offset parameter.

I've also noticed that the examples in the documentation have the mask and pattern parameters switched. I don't know if the function or the documentation should be fixed.
Comment 1 Carl-Anton Ingmarsson 2009-07-01 21:09:59 UTC
Created attachment 137707 [details] [review]
Patch with the above mentioned changes
Comment 2 Tim-Philipp Müller 2009-07-15 12:15:40 UTC
Marking as blocker, since it's new API and we need to decide what we want before we release it.

I think both the offset passed as argument and the offset returned should be relative to the current position. What would be the advantage of returning an offset relativ to the start of the data? Is it really a good idea to make the offset going in relative to the current position but then return an absolute offset as return value? Seems confusing to me.
Comment 3 Jan Schmidt 2009-07-15 13:15:28 UTC
I agree with everything Tim says, at all times
Comment 4 Carl-Anton Ingmarsson 2009-07-15 21:17:36 UTC
The reason I wanted to get the absolute position was that I used it in conjunction with gst_buffer_create_sub. I must admit that it probably makes sense to return the relative position and the code changes for me is anyway pretty small.

Nevertheless I think the docs should be clarified and the examples have the arguments switched.
Comment 5 Tim-Philipp Müller 2009-07-16 13:21:41 UTC
Ok, thanks. Let's WONTFIX this then. I've fixed up the docs:

 commit 79c8e2488513ce7aa84ae36742adaeba95182616
 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk>
 Date:   Thu Jul 16 14:17:03 2009 +0100

    docs: fix API docs for gst_{adapter|byte_reader}_masked_scan_uint32
    
    Clarify byte reader docs a bit: offset is relative to the current
    position of the reader, not to the start of the data. Also, the
    examples in both the adapter docs and the byte reader docs have
    the mask and pattern arguments swapped (see #587561). Spotted
    by Carl-Anton Ingmarsson.