562840 - Sometimes skip up to next keyframe at start of playback

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

Sometimes when we start playback of an ogg video, the new ogg decoder will skip playback of the frames up until the second keyframe. Our frame skipping logic is probably trigger happy...

Comment 1

[Chris Pearce [:cpearce (Not reading bugmail)]]

nsOggPlayStateMachine::DecodeLoop():

  if (audioPump && audioDecoded > audioPumpThresholdMs) {
    audioPump = PR_FALSE;
  }
  if (!audioPump && audioPlaying && audioDecoded < LOW_AUDIO_MS) {
    skipToNextKeyframe = PR_TRUE;
  }

So once we've decoded more than audioPumpThresholdMs of audio, we'll stop the audio pumping. If the audio push thread immediately pushes all data in the audio queue to the hardware, audioDecoded will be less than LOW_AUDIO_MS, and we'll go into frame skipping mode.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Attachment: Patch v1

* Change keyframe skip logic in DecodeLoop() to also take into account the amount of audio data which has been pushed to the hardware, but is still remaining to be played.
* Change nsBuiltinDecoderStateMachine::HaveNextFrameData() so that it also takes into account the audio data pushed to the hardware.
* Change nsBuiltinDecoderStateMachine::HaveNextFrameData() so that audio streams which finish before the video ends don't cause a transition to NEXT_FRAME_UNAVAILABLE (bug 564345).
* Ensure nsBuiltinDecoderStateMachine::mAudioEndTime is only accessed with decoder monitor held.
* If the audio push thread has more than 2000ms of audio queued ahead of the current playback position, sleep for 1000ms. Why? If we have lots of queued audio, the decode thread will sleep indefinitely, expecting to be woken up when the audio push thread pops off the audio queue. But if the audio queue is empty, the audio thread will also sleep indefinitely. This can happen when the audio thread has the just pushed the last packet, which isn't marked EOS (technically an invalid stream, likely an oggz-chopped file). This sleep I added works around this, coupled with a few NotifyAll()s in appropriate places.
* Moved the nsAudioStream::Drain() out of the audio push loop down to after it. We could sometimes miss it in cases where the decode loop slept right after decoding the last audio packet when the audio stream didn't contain an EOS packet.

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

http://hg.mozilla.org/mozilla-central/rev/2f2cff1c9bf2