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 568836 - [asfdemux] add seeking support when operating in push mode
[asfdemux] add seeking support when operating in push mode
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.10.10
Other Linux
: Normal normal
: 0.10.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-01-23 13:42 UTC by Hans de Goede
Modified: 2009-01-30 11:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
PATCH: add seaking support to asfdemux when in push mode (7.93 KB, patch)
2009-01-23 13:47 UTC, Hans de Goede
needs-work Details | Review
PATCH: Drop packages with an invalid replicated data length (690 bytes, patch)
2009-01-25 12:51 UTC, Hans de Goede
committed Details | Review
PATCH: add seaking support to asfdemux when in push mode (4.42 KB, patch)
2009-01-25 12:51 UTC, Hans de Goede
committed Details | Review

Description Hans de Goede 2009-01-23 13:42:15 UTC
Currently asfdemux does not handle seeking properly when operating in push mode, this patch fixes this.

When used together with (the accepted) patch from bug 469930, this enables seeking in mms and mmsh streams. I've tested this with several video and audio streams. I've also tested that this does not interfere in anyway with pull based operation.
Comment 1 Hans de Goede 2009-01-23 13:47:35 UTC
Created attachment 127092 [details] [review]
PATCH: add seaking support to asfdemux when in push mode

Note this (theoretically) also fixes the following from the TODO list:
* - _chain(): fix newsegment events for live streams where timestamps don't
*   start at zero (need sample files/streams for this)

Also note that with the msssrc patch, mmssrc now will always push a whole packet (or all headers the first push) at once. So sometime in the future (when the new mmssrc is widely used) this can be fixed too:

/* FIXME: eradicate ASF_FLOW_NEED_MORE_DATA */
Comment 2 Sebastian Dröge (slomo) 2009-01-23 19:33:04 UTC
Looks good from a short view... I'll take a closer look (and do some testing) tomorrow and probably will commit it then :)
Thanks Hans.
Comment 3 Sebastian Dröge (slomo) 2009-01-24 17:17:06 UTC
The patch looks fine but unfortunately doesn't apply to latest version in GIT. Could you update it? :)
Comment 4 Hans de Goede 2009-01-25 12:48:47 UTC
(In reply to comment #3)
> The patch looks fine but unfortunately doesn't apply to latest version in GIT.
> Could you update it? :)

Erm, !@#$, I thought that gst-plugins-bad-0.10.10 already had the push / pull based code paths unification patch (my fault). Anyways rebased (and retested) patch attached.

I've split it in 2, the first patch makes asfpacket.c drop (the current payload and the rest of) a packet when the replicated data length is invalid, instead of continuing with an invalid timestamp and an uninitialized payload.mo_size.

I did this because I noticed the timestamp was checked for validity in certain places, but used without checks in many others, so this seems the best thing to do, esp. given the warning already issued when this happens:
"invalid replicated data length, very bad" This warning does not sound like a good situation to keep processing the rest of the packet, hence my change to drop it.
Comment 5 Hans de Goede 2009-01-25 12:51:10 UTC
Created attachment 127197 [details] [review]
PATCH: Drop packages with an invalid replicated data length
Comment 6 Hans de Goede 2009-01-25 12:51:36 UTC
Created attachment 127198 [details] [review]
PATCH: add seaking support to asfdemux when in push mode
Comment 7 Sebastian Dröge (slomo) 2009-01-26 09:01:52 UTC
commit 3787e4b4a82d614b8f3850eb991577cc06326758
Author: Hans de Goede <jwrdegoede@fedoraproject.org>
Date:   Mon Jan 26 10:00:57 2009 +0100

    Add seeking support to asfdemux in push mode
    
    Fixes bug #568836.

commit 7a765c98bcf0a0d3667a4f7eaba96bb00a270033
Author: Hans de Goede <jwrdegoede@fedoraproject.org>
Date:   Mon Jan 26 09:57:26 2009 +0100

    Drop packets with an invalid replicated data length
    
    Drop packets with an invalid replicated data length
    instead of continuing with an invalid timestamp
    and uninitialized payload metadata.
    All other code assumes that the timestamps are valid.
Comment 8 Tim-Philipp Müller 2009-01-30 10:46:32 UTC
Do we need both demux->streaming and demux->push_mode? Shouldn't they be the same?
Comment 9 Hans de Goede 2009-01-30 11:10:59 UTC
(In reply to comment #8)
> Do we need both demux->streaming and demux->push_mode? Shouldn't they be the
> same?
> 

They actually are the same, my bad I hadn't noticed the streaming flag was already being used to track this, feel free to eradicate push_mode.