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 351659 - [wavpackparse] fix resync in push mode, implement it in pull mode
[wavpackparse] fix resync in push mode, implement it in pull mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.4
Assigned To: Tim-Philipp Müller
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-08-16 19:21 UTC by Sebastian Dröge (slomo)
Modified: 2006-08-18 21:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wavpackparse-1.diff (5.52 KB, patch)
2006-08-16 19:21 UTC, Sebastian Dröge (slomo)
none Details | Review
wavpackparse-1.diff (5.58 KB, patch)
2006-08-16 19:24 UTC, Sebastian Dröge (slomo)
committed Details | Review

Description Sebastian Dröge (slomo) 2006-08-16 19:21:02 UTC
Hi,
the attached patch fixes resync in push mode (g_strstr_len() doesn't look at the complete length but stops at \0) and implements it in pull mode.

Additionally the get_upstream_length() function uses query_peer_duration() instead of getting the peer itself, etc...

Bye
Comment 1 Sebastian Dröge (slomo) 2006-08-16 19:21:36 UTC
Created attachment 71034 [details] [review]
wavpackparse-1.diff
Comment 2 Sebastian Dröge (slomo) 2006-08-16 19:23:10 UTC
Oh, and it fixes event handling... pushing EOS and friends upstream again is not the best thing we can do ;)
Comment 3 Sebastian Dröge (slomo) 2006-08-16 19:24:48 UTC
Created attachment 71035 [details] [review]
wavpackparse-1.diff

fixed indention
Comment 4 Tim-Philipp Müller 2006-08-18 21:40:25 UTC
 - gst_wavpack_parse_find_marker(): add guard against size < 4

 - gst_wavpack_parse_resync_loop(): the sync logic isn't really safe:
   both times where you call gst_wavpack_read_header() you can't know/
   haven't checked that there is actually sizeof(WavpackHeader) data,
   so there are potential crashers here if you reach the end of the
   file and the len is < sizeof(WavpackHeader) or if there's a sync
   marker towards the end of the buffer without sizeof(WavpackHeader)
   data left.

 - also gst_wavpack_parse_resync_loop(): return flow return as
   return value, and initialize local variable to UNEXPECTED
   (*flow_ret was never set to anything if it bailed out right
   away, so the loop function would have checked an uninitialized
   flow value, no?)

 - s/wavpackparse/parse/ as variable name at least for new functions
   (makes code easier to read since lines are shorter and wrapped
   less often)


Committed with changes (let me know if the changes broke anything):

 2006-08-18  Tim-Philipp Müller  <tim at centricular dot net>

	Based on patch by: Sebastian Dröge <slomo at circular-chaos.org>

	* ext/wavpack/gstwavpackparse.c: (gst_wavpack_parse_sink_event),
	(gst_wavpack_parse_get_upstream_length),
	(gst_wavpack_parse_find_marker), (gst_wavpack_parse_resync_loop),
	(gst_wavpack_parse_loop), (gst_wavpack_parse_resync_adapter):
	  Fix resyncing in push mode not stopping re-syncing at embedded
	  zeroes; skip garbage between frames in pull mode as well if
	  necessary; use gst_pad_query_peer_duration(); push EOS and
	  NEWSEGMENT event in right direction (#351659).