GNOME Bugzilla – Bug 724633
oggdemux: ignores last page in push mode
Last modified: 2014-02-20 20:43:57 UTC
Created attachment 269540 [details] result of dumping the ogg file This is a follow up of bug 724509. After using my patch, flac works fine, but the converted ogg still dumps not quite correctly. preroll says it's S16LE mono, but 'aplay -r 11025 -f S16' doesn't sound right and is only 22528 bytes long, which is obviously too short for original 22491 mono samples.
Easy way to reproduce the issue: Get the ogg encoded version and do: gst-launch-1.0 playbin uri=file:// vs gst-launch-1.0 playbin uri=pushfile://
Created attachment 269661 [details] [review] oggdemux: allow file to go until the end in push mode When seeking back to original state after duration seeks, let upstream know that we want the whole file, including the last byte that wasn't requested on the duration seeks.
Review of attachment 269661 [details] [review]: Please also backport to 1.2 :)
(In reply to comment #3) > Review of attachment 269661 [details] [review]: > > Please also backport to 1.2 :) On this note: what about patches from bugs earlier in this chain ?
(In reply to comment #3) > Review of attachment 269661 [details] [review]: > > Please also backport to 1.2 :) I want to ponder a bit more on the oggdemux code. I'm afraid this patch will reset and previous segment.stop configured on it. Need to read its code a little bit more to find a solution.
No it does not :) It is always reset to 'duration-1' anyway when doing the whole 'seek to get duration' trick and I can't think of any use case to make it worth preserving upstream bytes segment across those seeks. So I'm pushing this patch. commit b0985365affd8f469a288ca7eecb620007d3b1fa Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Wed Feb 19 01:55:50 2014 -0300 oggdemux: allow file to go until the end in push mode When seeking back to original state after duration seeks, let upstream know that we want the whole file, including the last byte that wasn't requested on the duration seeks. https://bugzilla.gnome.org/show_bug.cgi?id=724633
Pushed to 1.2 commit 913ac219eff202560b7aa36945be24cc23e80a71 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Wed Feb 19 01:55:50 2014 -0300 oggdemux: allow file to go until the end in push mode When seeking back to original state after duration seeks, let upstream know that we want the whole file, including the last byte that wasn't requested on the duration seeks. https://bugzilla.gnome.org/show_bug.cgi?id=724633
This whole 'do not read the last byte' thing to get the duration in push mode seems very fragile if not wrong to me. Why is this done again, so it doesn't do EOS stuff? If yes, we should maybe prevent that some other way, e.g. by adding a seek flag or so. The fix here with the SEEK_TYPE_SET, end=-1 is very unintuitive, and I'm sure it will get 'fixed' sooner or later ;)
(In reply to comment #8) > This whole 'do not read the last byte' thing to get the duration in push mode > seems very fragile if not wrong to me. Why is this done again, so it doesn't do > EOS stuff? If yes, we should maybe prevent that some other way, e.g. by adding > a seek flag or so. The fix here with the SEEK_TYPE_SET, end=-1 is very > unintuitive, and I'm sure it will get 'fixed' sooner or later ;) The +1 at the beginning and -1 at the end are used to prevent libogg from detecting a valid page and outputting something IIRC. Yes, I agree this all sounds a bit ugly. We could likely track oggdemux own seeks/segments using seqnums and use a segment query to restore the origial segment afterwards.