Sometimes skip up to next keyframe at start of playback

RESOLVED FIXED

Status

()

Core
Audio/Video
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: cpearce, Assigned: cpearce)

Tracking

Trunk
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
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...
(Assignee)

Updated

8 years ago
Assignee: nobody → chris
Status: NEW → ASSIGNED
(Assignee)

Comment 1

8 years ago
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.
(Assignee)

Comment 2

8 years ago
Created attachment 444030 [details] [diff] [review]
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.
Attachment #444030 - Flags: review?(chris.double)
(Assignee)

Updated

8 years ago
Blocks: 564345
(Assignee)

Updated

8 years ago
Attachment #444030 - Flags: review?(chris.double) → review?(kinetik)
Attachment #444030 - Flags: review?(kinetik) → review+
(Assignee)

Comment 3

8 years ago
http://hg.mozilla.org/mozilla-central/rev/2f2cff1c9bf2
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.