GNOME Bugzilla – Bug 585956
[mp3parse] indexing improvements
Last modified: 2009-06-22 09:48:09 UTC
+++ This bug was initially created as a clone of Bug #585576 +++ There was this nice fix for mp3parse that it doesn't build an index table over time when it cannot seek anyways, which is supposed to stop the unbounded memory growth when listening to internet radio (for embedded devices, extra CPU load is also a factor). Problem is that souphttpsrc initially assumes that servers support seeking. Try: GST_DEBUG=mp3parse:4 gst-launch-0.10 playbin2 uri=http://gffstream.ic.llnwd.net/stream/gffstream_stream_wdr_einslive_a 2>&1|fgrep index The server sends no Content-Length header, so there is no stream duration (which makes it questionable to assume seekability(?)). Either souphttpsrc should have better prediction of seekability or maybe mp3parse should suppress index table building when the upstream duration query fails.
Created attachment 136703 [details] [review] mp3parse: assume seekability only if we know the upstream size
Created attachment 136704 [details] [review] mp3parse: don't put every single frame into the index
I think you're leaking the seeking query here (in the first patch), otherwise looks good. Second patch is probably ok too, but I find that code fairly impenetrable already, so I'm not 100% sure.
> I think you're leaking the seeking query here Fixed, thanks. commit af3ab2ae945325976cb247e819a348850b81cd8f Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Tue Jun 16 09:45:59 2009 +0100 mp3parse: don't put every single frame into the index Let's not put every single mp3 frame in our index, a few frames per second should be more than enough. For now use an index interval of 100ms-500ms depending on the upstream size, to keep the index at a reasonable size. Factor out the code that adds the index entry into a separate function for better code readability. commit 1db592839e1cb07fca7bbd3f2878e76bfb154ac2 Author: Tim-Philipp Müller <tim.muller@collabora.co.uk> Date: Tue Jun 16 01:40:42 2009 +0100 mp3parse: assume seekability only if we know the upstream size While technically upstream may be seekable even if it doesn't know the exact size, I can't think of a use case where this distincation is relevant in practice, so for now just assume we're not seekable if upstream doesn't provide us with a size. Makes sure we don't build a seek index when streaming internet radio with sources that pretend to be seekable until you try to actually seek.