GNOME Bugzilla – Bug 638276
oggstream: when the last keyframe position is not known, do not use -1
Last modified: 2011-01-06 22:34:58 UTC
Instead, use either 0 or 1, depending on bitstream version, which give the correct result for streams which aren't cut off at start. This allows that function to not return negative granpos.
Created attachment 177194 [details] [review] oggstream: when the last keyframe position is not known, do not use -1
Ok, so this looks ok, doing bit ops with negative values is not going to yield sensible things, but is there another bug present here? (Why do we not know the last keyframe pos here?) If this may still cause problems under certain circumstances, it would be good to file another bug with more details so we don't forget about it..
Apparently, this can happen for 'interpolated' granpos from oggdemx. At start, before oggdemux locks on the first keyframe, I had interpolated granpos 0, 1, 2, 3. Later, it would use the 'real' ones. I looked a bit where these came from, but, er, it's a bit scary, and I stopped looking when I realized it was regenerating granpos, but it only happened (AFAICS) for the first few packets.
The only problem in granulepos regeneration that I know of is for theora streams where several frames including a key frame are included in the first theora data page. Thus, for a stream that starts out with a page like this: (frame type) (original frame gp) non-key 1000|10 non-key 1000|11 keyframe 1012|0 non-key 1012|1 -> page gets granulepos 1011|2 The reconstructed gp values are: non-key garbage (1, iirc) non-key garbage (2, iirc) keyframe 1012|0 non-key 1012|1 I didn't spend much time on this when I discovered it, because it only affected remuxing and not playback. There are two possible fixes: Drop the packets before the first keyframe. Or, create more reasonable bogus granulepos values for the pre-keyframe packets. But one needs to also check for the case: keyframe ??? (should be 1|0) non-key ??? (should be 1|1) keyframe 3|0 non-key 3|1 Note that we now have a method for checking if a packet is a keyframe, which can be used instead of simply checking for "|0" in the granule pos.
#619778 is related to the theora granulepos regeneration bug.
As far as I can make it, the code in oggdemux.c at line circa 820 finds a first page with several packets ending on it, and starts rewriting granpos from an "accumulated granule" counter. At this time, the location of the previous keyframe is unknown (but it is the first packet, though this code does not know it yet as it has not seen the granpos yet). This is on the "Patent absurdity" video.
I fixed the underlying bug in bug #619778 (please check). The code around 820 doesn't do exactly what it says: the recreated granulepos is magically correct in some common cases, but is never actually used. It is recreated *again* when the buffer is pushed.
commit b03b223fb110e3f5f2dcfc476b6db8ff1a205380 Author: Vincent Penquerc'h <vincent.penquerch@collabora.co.uk> Date: Wed Dec 29 15:27:44 2010 +0000 oggstream: when the last keyframe position is not known, do not use -1 Instead, use either 0 or 1, depending on bitstream version, which give the correct result for streams which aren't cut off at start. This allows that function to not return negative granpos. https://bugzilla.gnome.org/show_bug.cgi?id=638276