GNOME Bugzilla – Bug 469930
[mmssrc] seeking support PATCH
Last modified: 2009-02-24 00:30:38 UTC
Sometime ago I stumbled over an libmms patch which Debian has included which adds seeking support to libmms. After a couple of 10 hour days learning gstreamer and working on this I'm happy to report that I can now seek in totem in the mms streams from the Dutch television. First of all this requires 3 patches to libmms: libmms-0.3-seek.patch: Seek patch from debian ported from libmss-0.2 to 0.3 libmms-0.3-64bit-off_t.patch: Fix the fact that the Debian patch needed apps/libs using libmms to be compiled with -D_FILE_OFFSET=64 libmms-0.3-length.patch: Lie about the file size as we cannot get to the ASF index object at the end of most asf files through mms So maybe gstreamer-plugins-bad should come with its own libmms copy until this gets picked up upstream (as far as libmms upstream is still alive). Then there is the actual gst seeking support patch: gst-plugins-bad-0.10.5-mms-seek.patch Last there is also a patch to the asf demuxer from gst-plugins-ugly, let me know if you want that in another bug report: gst-plugins-ugly-0.10.6-asf-seek.patch This patch isn't strictly needed, actually without it its much easier to trigger the frozen video problem described below. It does result in a much better seeking experience though, so once the frozen video issue is fixed, it should definitively get in. With all this in place seeking works, but there are a couple of caveats: -gstreamer and / or totem now want to run the typefind helper on the stream, which causes a long initial delay when playing a stream. Setting gstbasesrc type_find property to 0 is not enough to stop this, I need some help fixing this. And a much bigger one: -at work (with _high_ speed internet, the 100mbit switch is the bottleneck), it works well. But at home most of the time after a seek the playback doesn't start. Totem shows the first frame of the seeked to part, buffers data and then nothing happens, the seconds don't start counting, nothing. totem is still 100% responsive to ui input though, pause/unpause doesn't help. Seeking to another place does result in a new first frame appearing, new buffering and usually again nothing happening after this. Sometimes it does start to play then. This seems a timing issue, where some part of the gstreamer stack and / or totem stops when the new packets don't come quickly enough after the seek. I've run totem with "--gst-debug=mmssrc:5,asfdemux:5 --gst-debug-no-color &> log" and asfdemux keeps pulling and getting packets from mmssrc, and also keeps pushing both audio and video packets to its sinks. But somewhere further downstream things seem to go wrong, because as explained in totem things stop, totem doesn't freeze but the video in it does. I'm afraid my knowledge of gstreamer isn't sufficient here and I would be much obliged if you could help. I think this is a bug elsewhere, but since sofar asf seeking was only done from files, which can deliver data pretty quickly, never got triggered. Or it could be a locking issue in my code. Either way I need help.
Created attachment 94265 [details] [review] libmss seeking patch
Created attachment 94266 [details] [review] libmms don't require special cflags for users patch
Created attachment 94267 [details] [review] libmss length reporting patch
Created attachment 94268 [details] [review] gstmmssrc seeking patch
Created attachment 94269 [details] [review] gstasf better seeking patch
I just had a small break through, if I first pause the stream, then seek and wait for the buffering at the new location to complete and then press play, seeking seems to work.
Good news after updating to gstreamer and gst-plugins-base 0.10.15 the problem of hanging after seeking is gone. I've also found a way around the long initial delay caused by typefinding randomly seeking through the stream. So my patch works 100% now. In the mean time I've also added mmsh seeking support to libmms. Last I've added a set of mmsx_foo function too libmms of which the connect() first tries a mms connection and then fallsback to mmsh, the other mmsx_foo functions then forward the foo() call to either mms or mmsh this saves a lot of if ... else ... in gstmmssrc.c making the code more readable. There is one problem though, the bulk of the work has happened in libmms not gstmmssrc, which is good, but means that it cannot be tested nor used without having a patched libmms. A patched libmms is available from livna devel (libmms-0.3-5), or you can get all the patches from here: http://people.atrpms.net/~hdegoede/libmms-patches/ Unfortunately libmms upstream is dead. I'll be contacting them to try and take maintainership over from them, and if they don't respond I'll file a takeover request with the sourceforge admins. Alternatively gstreamer-plugins-bad could include its own copy, but thats not the best solution IMHO. I'll attach a patch against gstreamer-plugins-bad cvs implementing mms/mmsh seeking in gstmmssrc and one against gstreamer-plugins-ugly to make gstasfdemux better handle the fact that there are no time indexes available through mms(h) with this patch gstasfdemux asks its source to do time -> byte offset conversion when there are no indices before using the current kludge for this, and as it happens gstmmssrc can do time -> byte offset conversion now :)
Created attachment 100751 [details] [review] Patch adding mmssrc seeking support (needs new libmms)
Created attachment 100753 [details] [review] Patch making asfdemux better handle seeking in files without index, when that file is streamed through mms
libmms-0.4 incorperating all my seeking work is now available as an official libmms release: http://sourceforge.net/projects/libmms So now all that is needed is to patch configure to require libmms >= 0.4 for mmssrc, review and apply my 2 patches to gstreamer and we have seeking support! Anything I can do to speedup the review process?
Hm, there's no need to switch away from GstPushSrc and use GstBaseSrc directly if you want seeking support. MMS is still a "push" based protocol, right? ;) Look at how the soup and neon plugins do it for example.
The ASF patch looks correct though, committed. (Well, it would also work to send a TIME based seek upstream instead of doing the CONVERT query, right?) 2008-02-22 Sebastian Dröge <slomo@circular-chaos.org> Patch by: Hans de Goede <j dot w dot r dot degoede at hhs dot nl> * gst/asfdemux/gstasfdemux.c: (gst_asf_demux_handle_seek_event): If we don't have the position to seek to in our index first try to convert from TIME to BYTES upstream and only if that fails too use the old hack to simply seek to an earlier position and let the sink drop everything before segment start. Partially fixes bug #469930.
(In reply to comment #11) > Hm, there's no need to switch away from GstPushSrc and use GstBaseSrc directly > if you want seeking support. MMS is still a "push" based protocol, right? ;) > > Look at how the soup and neon plugins do it for example. > So, to be clear, you want the patch to be changed to: -not change the base class from pushsrc -> basesrc -implement a do seek function todo the actual seeking, instead of seeking in create as its done in the current patch Correct? (In reply to comment #12) > The ASF patch looks correct though, committed. (Well, it would also work to > send a TIME based seek upstream instead of doing the CONVERT query, right?) > Not at the moment, as currently except for supporting time -> bytes conversion queries mmssrc still is completely bytes based and still only supports bytes based seeking. Seeing how tightly mms <-> asf are married and how handling seek queries is already done at the asf level I didn't see this as a problem, as afaik the only downstream of mms will ever be asf and that already handles seek queries, but maybe its trivial to add support for seekqueries somehow, letting most of the work be done by basesrc? Note: although I'm very interested in gstreamer development I'm as green as gras when it comes to gstreamer internals like these.
Yes, I'd prefer to not change from pushsrc to basesrc and keep pushsrc. And for seeking you have to implement a do_seek() function, yes. You then can either seek directly in there or update the offset in your instance struct and do the seeking in create() if last_offset!=new_offset. The first one would be preffered if the libmms seeking happens instantly and without blocking, the second would be preffered otherwise (as it will block the UI when seeking if your seek blocks, for example)
(In reply to comment #14) > Yes, I'd prefer to not change from pushsrc to basesrc and keep pushsrc. > > And for seeking you have to implement a do_seek() function, yes. You then can > either seek directly in there or update the offset in your instance struct and > do the seeking in create() if last_offset!=new_offset. > > The first one would be preffered if the libmms seeking happens instantly and > without blocking, the second would be preffered otherwise (as it will block the > UI when seeking if your seek blocks, for example) > Ok, Quite some time passed since I've done these patches, so I've been busy re-familiarizing myself with the asfdemux code. The reason why I did things the way I did them, is because keeping mmssrc as a push src and using do_seek, will only work if the asfdemux code starts passing seek query's to its src when operating in push mode, the current seek code in asfdemux only works in pull mode and I'm not sure how easy it is to fix this as asfdemux is somewhat convoluted IMHO: There seem to be 2 almost completely different implementations for push and for pull mode, which seems totally unnecessary, since when in push mode one can just buffer data until a complete packet has been received and then dive into the same code path used to process a packet when operating in pull mode. The only reason I can see for having seperate push mode code, is because a packet can contain multiple video frames and audio fragments, and you want to push them as soon as you've received enough of a packet to push a frame / audio fragment. But that is _only_ usefull when streaming over plain http, as mms and mssh work packed based and libmms if it doesn't have all the requested data buffered will always read a whole packet into its internal buffer before returning from a read call, even if you've asked for less. When I started working at the mms seeking I've seriously considered completely ripping out the seperate push based asf demux code, but in the end I didn't get around to that, after a full week of working on this I was happy to have things working as well as I have them now. Notice that currently mmssrc with my patches offers random access for non live streams, so asfdemux will operate in pull mode for non live streams. And this seems to work fine for all the Fedora users using livna, so thats upto about 1 million people, which IMHO is proof that there really is no reason to have fully seperate code-paths in asfdemux for pull / push based mode.
> There seem to be 2 almost completely different implementations for push > and for pull mode [in asfdemux], which seems totally unnecessary, (...). This is mostly for historic reasons: I rewrote the packet parsing code from scratch when I added pull-mode support to asfdemux. I meant to change the push mode code path to use the new packet parsing code as well, but haven't done so yet for two reasons: (a) an -ugly release was supposedly imminent for a long time, so I didn't want to do huge changes before the upcoming release, and (b) there were some open issues at the beginning of some streams that I wanted to look into (I believe those were due to older libmms versions dropping the first packet though). I'll have a look if I can find the patch for this now that -ugly has been released.
(In reply to comment #16) > (b) > there were some open issues at the beginning of some streams that I wanted to > look into (I believe those were due to older libmms versions dropping the first > packet though). Yes I'm pretty sure that those issues at the beginning of the streams where a libmms bug, which is fixed in libmms-0.4
(In reply to comment #16) <snip> > I'll have a look if I can find the patch for this now that -ugly has been > released. Found the patch?
Given that recently (see bug 560348) the push and pull based code paths in asfdemux have been merged, can we please fix this? I know my currently attached patch which claims that msssrc is pull based is a hack, (but it has served well to avoid the pull based code mess in asfdemux). Unfortunately my gstreamer internals knowledge is not so well, so I think it would be good for a more experienced gstreamer dev to write a proper patch. I will happily test this and answer any questions about how seeking is handled in libmms (I've worked quite a bit on the libmms seeking code to get seeking to work in my pretend to be a pull based src patch).
Created attachment 127078 [details] [review] Patch adding mmssrc seeking support (needs libmms >= 0.4) Here is a new version of my patch, which is no longer a hakc, but makes mmssrc correctly handle push based seeking. Unfortunately asfdemux does not handle this well (not all). I've been making good progress there too, but my patch needs more work, I hope to be able to supply a new patch soon.
commit 20b715ac7970681bd38ce26c4b8f0a076229a88c Author: Hans de Goede <jwrdegoede@fedoraproject.org> Date: Fri Jan 23 11:50:29 2009 +0100 Add seeking support to mmssrc. Fixes bug #469930. Add proper seeking support to mmssrc and clean up some code. This requires libmms >= 0.4.
*** Bug 572613 has been marked as a duplicate of this bug. ***