GNOME Bugzilla – Bug 580133
Regression in baseparse since last release
Last modified: 2009-05-03 12:54:15 UTC
Commit ad2c7bffe7d94c79184904b14101cbc93eeb9ebd breaks the unit tests. If I interpret the problem correctly, the old logic sent a newsegment, and then flushed any data in the adapter (presumably because it comes from the old segment). The new logic flushes the adapter whenever there is a pending newsegment event, but doesn't send the newsegment until there is a complete frame. This leaves it in a state where until there is a single input buffer containing a full frame, the parser continuously flushes the adapter (containing part of a frame), and never pushes the newsegment event. See gstbaseparse.c:886 for the offending gst_adapter_clear() code. I would guess that the fix is to move the gst_adapter_clear() to a better location, possibly when the pending newsegment event is enqueued in the first place - as presumably any data after then comes from the new segment and needs to be separated from the old, incomplete, frames.
Created attachment 133447 [details] [review] Possible patch Patch newsegment handling as suggested; looks reasonable and fixes unit test.
There is some weird logic about configuring the pending segment in pull-mode in gst_base_parse_handle_seek(). I'm not sure it'll clear the adapter in that path now, but I don't follow the intent of the code well enough to be sure.
It looks like pretty standard seek handling that prepares some newsegment events for the streaming thread (e.g. as in basesrc). As far as I can tell/read/search/grep, the adapter is not used in pull mode, so that should be OK.
Unless alternatives or objections, I'll be pushing the above fix prior to upcoming -bad freeze.
commit 9bbacae78f5750825bd79ec332c0ff0105552c0f Author: Mark Nauwelaerts <mark.nauwelaerts@collabora.co.uk> Date: Mon Apr 27 22:39:15 2009 +0200 baseparse: fix (regression in) newsegment handling (aacparse, amrparse, flacparse). Fixes #580133.