GNOME Bugzilla – Bug 308312
Make mp3parse plugin implement GST_SEEK_FLAG_ACCURATE
Last modified: 2008-06-28 03:59:04 UTC
when seeking and playing an mp3 file with the mad plugin, both seeking and playing is quite inprecise.
Created attachment 48002 [details] sample code demonstrating the problem see the sample program demonstrating the problem. usage: ./seek <mp3 filename> <seek to secs> [<play until secs>]
for example, running the sample code on a 5 second, fixed bitrate mp3 file gives me the following output: ./seek /tmp/5seccounter.mp3 2 tried to seek to: 2000000000 time after seek: 2193696145 seek error: 193696145 played until: 4786553287 total time played: 2592857142 intended playtime: 3070125000 playtime error: -477267858 file total playtime: 5070125000 which means that seeking was off by 10% (.2 secs), and it didn't play the last .2 secs of the file. thus the intended playtme of 3 seconds shrunk to 2.6 second, which is over 10% in error
I'm ok with mad implementing precise seeking, but not by default. As you may have noticed, we have a flag GST_SEEK_FLAG_ACCURATE, and that is intended for such behaviour. So make mad implement that flag and use it in your application, and you're set.
Akos, any word on this? Could you port it to 0.10?
seems like the same inaccuracy in 0.10 didn't yet have time to implement a more accurate solution :(
Cool. Well if you get back to it you might want to look at Jan's mp3 plugin as well -- just a thought.
Akos, after chatting with Jan, the proper way to implement this behaviour: * Generate an index of the mp3 frames [offset, time, duration] as you go along. This works both for push-based and pull-based behaviours. * If you want to seek back to a certain position, you know which frame you need, for which you can request upstream for the frames' time of offset. * If you want to seek to a position you haven't parsed yet, you keep parsing the incoming frames (without decoding them) until you have the frame containing the requested seek start position. For both behaviours (seek back or forward), you will have to crop the decoded frame so it starts from the requested seek start position. Hopes this helps
*** Bug 345971 has been marked as a duplicate of this bug. ***
Fixing this would be very good as it would improve transcoding with Pitivi a lot. There are tons of movie clips out there using mp3 audio and without this working correctly Pitivi borks on transcoding them.
Not wanting to just add a Me Too! entry here, but this would be really useful for Jokosher too for a number of areas.
This is a very popular Jokosher bug[1], and I have linked here from the Jokosher bug in launchpad. As more people use Jokosher and Pitivi this bug is going to become a real pain for users. I am sure everyone is aware of how popular mp3s are. [1]https://launchpad.net/jokosher/+bug/73327
Note that Jokosher bug[1] occurs also using Fluendo MP3 decoder plugin. I tried it just now. [1]https://launchpad.net/jokosher/+bug/73327
*** Bug 395054 has been marked as a duplicate of this bug. ***
Updated title of this bug to talk about mp3parse instead of mad. With the new mp3parse plugin in place it only needs to be fixed there and it will work with mad, fluendo mp3 and probably also ffmpeg mp3.
Created attachment 91270 [details] [review] mp3-accurate-seek.diff The attached patch somewhat implements accurate seeking but still has some problems, especially the segment handling. Also, for testing purposes, every seek with that patch will be an accurate seek. Would be nice if someone could look at it, give suggestions, ideas, find bugs, etc...
Created attachment 91369 [details] [review] mp3-accurate-seek.diff Ok, this does it it seems... only problem I see now is that at least the mad plugin doesn't clip the decoded buffers so only the sink will do the clipping. This should be implemented in mad ASAP, I'll look into it. Also one could improve the "frame_offset" variable by actually looking at the mp3 frames and calculating the real amount of frames that are necessary to decode the one we want. 9 (or 29) as the theoretical maximums are working fine though.
Created attachment 91442 [details] [review] mad-buffer-clipping.diff Patch to make mad clip it's buffers appropiately. With these two patches we have accurate time seeking for MP3.
Created attachment 91443 [details] [review] mad-buffer-clipping.diff a bit of cleanup
Created attachment 91444 [details] [review] mp3-accurate-seek.diff
*** Bug 455477 has been marked as a duplicate of this bug. ***
Created attachment 91697 [details] [review] mad-buffer-clipping.diff new versions, should be perfect now. What do you think, Edward?
Created attachment 91698 [details] [review] mp3-accurate-seek.diff (well, every seek is accurate still but this will be changed before committing)
works perfectly here. Only issues I can see are in fact with mpeg streams (using mpegdemux)... but that's another matter altogether. COMMIT !
Perhaps we should consider adding a function like the mad clip_buffer one to the gstaudio library as a helper function - we're certainly making enough copies of it...
I'll care for the clip_buffer function in gstaudio later, good idea. These two patches are committed now... flump3dec still needs buffer clipping though... someone here who wants to implement it? :) 2007-07-13 Sebastian Dröge <slomo@circular-chaos.org> * gst/mpegaudioparse/gstmpegaudioparse.c: (gst_mp3parse_reset), (gst_mp3parse_init), (gst_mp3parse_dispose), (gst_mp3parse_sink_event), (mp3parse_seek_table_last_entry), (gst_mp3parse_emit_frame), (gst_mp3parse_chain), (mp3parse_handle_seek), (mp3parse_src_query): * gst/mpegaudioparse/gstmpegaudioparse.h: Implement accurate seeking in mpegaudioparse. Fixes #308312. Also implement segment seeks. 2007-07-13 Sebastian Dröge <slomo@circular-chaos.org> * ext/mad/gstmad.c: (_do_init), (gst_mad_init), (index_seek), (normal_seek), (gst_mad_sink_event), (clip_outgoing_buffer), (gst_mad_chain), (gst_mad_change_state): * ext/mad/gstmad.h: Implement buffer clipping and use GST_BOILERPLATE instead of manual GType magic. Part one of bug #308312.
The gstaudio clip_buffer function is now bug #456656 if someone cares.
*** Bug 373190 has been marked as a duplicate of this bug. ***