The new Ogg backend has includes code from the original backend that was needed to workaround a liboggplay bug. This causes a seek after metadata reading. This should be removed.
Comment on attachment 445879 [details] [diff] [review] fix This actually breaks seeking over HTTP. We use nsBuiltinDecoderReader::FindStartTime() inside nsBuiltinDecoderReader::GetBufferedBytes() in order to determine what time ranges are buffered. If we remove that seek, FindStartTime() returns incorrect values, and our seek tries to look for the seek target inside the wrong range, and gets confused. Sorry, when we were discussing this I totally forgot that we reused FindStartTime() this way! Perhaps we should instead move the seek out of nsBuiltinDecoderReader::FindStartTime() and into nsBuiltinDecoderReader::GetBufferedBytes(), and remove the aOffset parameter from nsBuiltinDecoderReader::FindStartTime()? Then we wouldn't be doing the seek after loading metadata, and we'd still be able to seek in buffered ranges.
Odd that all ours tests pass with this patch applied. Are out tests not testing seeking correctly?
This moves the seek from the nsBuiltin* start and end finders into the Ogg backend itself. I've done the minimum needed to move it out of the base classes and into Ogg. Longer term I think we need to refactor the way this is done. The FindStart/End and GetBufferedBytes stuff should be moved into the Ogg backend. We can then get rid of mDataOffset from the VideoData structure. I'll raise a bug for this.
Comment on attachment 446126 [details] [diff] [review] fix Looks good.
Attachment #446126 - Flags: review?(chris) → review+
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.