[Bug 621897] [oggdemux] reports wrong duration

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=621897
  GStreamer | gst-plugins-base | git

Mark Nauwelaerts <mnauw> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mnauw at users.sourceforge.net

--- Comment #9 from Mark Nauwelaerts <mnauw at users.sourceforge.net> 2011-04-21 13:20:32 UTC ---
Branch mentioned above handles quite some of these cases/bugs; some review
comments (mainly on the first commit, unaffected by second commit):

* a few changes seem included that may be unrelated to the (duration/push seek)
enhancements, e.g. the discont marking in gst_ogg_pad_stream_out, or dropping a
_free in gst_ogg_demux_deactivate_current_chain.  If so, they best go in a
separate commit, rather than drowned in this one.

* the QUERY_SEEKING case in gst_ogg_pad_src_query now has 2 identical if (...)
tests in a row

* time info in debug statements is provided here with %f, customary is to use
GST_TIME_FORMAT and GST_TIME_ARGS

* some debug only segments might do with #ifndef GST_DISABLE_GST_DEBUG

* don't know what gst-indent is (not) doing here, since it does not "complain"
but there are quite some code style issues, e.g. many lines exceeding 80 chars;
comments, debug strings, and also code in gst_ogg_demux_get_duration_push, or
no blank line following declaration in e.g. gst_ogg_demux_check_duration_push 

* also // comments left here or there (along with TODO)

* code chunk added to gst_ogg_demux_chain_peer to determine duration might go
nicer in gst_ogg_demux_submit_packet where new segment event is setup, as it is
also an opportunity to determine the chain->segment_stop there

* comment in gst_ogg_pad_submit_page about "queue blocking" and eos handler is
unclear; sending a flushing seek upstream should take care of queue in between
(see also later)

* in gst_ogg_demux_perform_seek_push:
- various state variables are manipulated (e.g. push_state) without any
locking, though they might be accessed by streaming thread in the meantime
without being setup fully/properly.  Seems best to do as little possible to
these at that time, and only send seek upstream and perform remaining stuff
when that comes in (newsegment).
- also, validity of all data may not be checked (already push_start_time by
then ? etc)
- it may be easier to keep the seek event around rather than all individual
bits and pieces of it (i.e. flags, start etc)
- worse, using the original seek event's start_type, stop_type, etc in an
upstream BYTE seek later on is wrong; the upstream BYTE seeks always simply
target a specific offset
- moreover, the only kind of seek that is really handled properly in push mode
is a flushing seek with seek_type_set start time (and usually open-ended stop
time), cfr other demuxers or baseparse.  So, better check accordingly and only
seek if in such a case.  In particular, only handling flushing cases may help
(a.o.) with the queue situation above.
- [minor nitpick] on the other hand, checking for a PUSH_DURATION in progress
should not be needed, i.e. it should handle being interrupted (particularly by
flushing seek), though that may need some other variable caution

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.