Playback video only shows keyframes.

RESOLVED WORKSFORME

Status

()

Core
Audio/Video
P2
normal
RESOLVED WORKSFORME
3 years ago
3 years ago

People

(Reporter: jya, Assigned: mattwoodrow)

Tracking

(Blocks: 1 bug)

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

3 years ago
From time to time, only noticing it with YouTube with MSE enabled, video playback starts to freeze and only keyframes are now displayed.

Simply commenting skipToNextKeyFrame = true; in MediaDecoderStateMachine prevents the issue from occurring.
Include Benjamin who might be interested.
Priority: -- → P1
(Reporter)

Updated

3 years ago
Depends on: 1112445

Updated

3 years ago
Blocks: 1118945
(Assignee)

Comment 2

3 years ago
I was seeing this on my nightly, but it went away after restarting for an update. I haven't been able to reproduce in my dev builds.

It could be related to load from background tabs?
Marcia - I don't think this is MSE specific. Can you see if you can reproduce this on release and compare that with nightly? I think it is easier to reproduce on slow machines. You probably want to set media.windows-media-foundation.use-dxva=false or layers.acceleration.disable=true to force software paths.
Flags: needinfo?(mozillamarcia.knous)
We believe that this happens more on Mac because we don't drain the re-order queue when we hit a keyframe. Doing so effectively makes the playback queue longer.
Assignee: nobody → matt.woodrow
No longer blocks: 1118945
(Assignee)

Comment 5

3 years ago
Created attachment 8547344 [details] [diff] [review]
Drain reorder queue once we've decoded all frames up to a keyframe
Attachment #8547344 - Flags: review?(jyavenard)
(Reporter)

Comment 6

3 years ago
Comment on attachment 8547344 [details] [diff] [review]
Drain reorder queue once we've decoded all frames up to a keyframe

Review of attachment 8547344 [details] [diff] [review]:
-----------------------------------------------------------------

I'm not convinced it serves any purposes (nor that it will solve the issue about MDSM skipping to the next keyframe).

This will also output of order frames under some conditions. Try with the following patch (that assert if we output out of order frames)

http://people.mozilla.org/~jyavenard/mediatest/BreakVDA-mp3.mp4

::: dom/media/fmp4/apple/AppleVDADecoder.cpp
@@ +39,5 @@
>    , mTaskQueue(aVideoTaskQueue)
>    , mCallback(aCallback)
>    , mImageContainer(aImageContainer)
> +  , mLastInput(0)
> +  , mDrainOnDecode(0)

needs to be initialised to -1 ; otherwise DrainReorderedFrames will be unnecessarily called twice when the first I-frame with a PTS of 0 is decoded

@@ +315,5 @@
> +      DrainReorderedFrames();
> +    } else {
> +      mDrainOnDecode = mLastInput;
> +    }
> +  }

This needs to put into a function so we don't have duplicates

@@ +318,5 @@
> +    }
> +  }
> +  mLastInput = aSample->composition_timestamp;
> +  mPendingDecodes++;
> +

Not too important at this point as error are fatals. But mPendingDecoder should be decremented should an error occurred.

::: dom/media/fmp4/apple/AppleVTDecoder.cpp
@@ +219,5 @@
> +      mDrainOnDecode = mLastInput;
> +    }
> +  }
> +  mLastInput = aSample->composition_timestamp;
> +  mPendingDecodes++;

Same comments as above
Attachment #8547344 - Flags: review?(jyavenard) → review-
(Reporter)

Comment 7

3 years ago
Created attachment 8547573 [details] [diff] [review]
Check pts order (with proposed code)
(Reporter)

Comment 8

3 years ago
Created attachment 8547574 [details] [diff] [review]
test order (with original code
(Reporter)

Updated

3 years ago
Attachment #8547573 - Attachment description: Check pts order (with original code) → Check pts order (with proposed code)

Comment 9

3 years ago
Is there a reason this should happen more with 60fps videos? Because something that looks like it might be this happnens when I seek into unbuffered areas. Practically all the time. Meanwhile with 30fps videos I never have this issue.
It "fixes itself" after a while if I keep seeking back and forth. (without going out of the buffered range)
(Assignee)

Comment 10

3 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #6)
> Comment on attachment 8547344 [details] [diff] [review]
> Drain reorder queue once we've decoded all frames up to a keyframe
> 
> Review of attachment 8547344 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I'm not convinced it serves any purposes (nor that it will solve the issue
> about MDSM skipping to the next keyframe).

The theory is that decoding keyframes takes significantly longer than decoding the others.

While this long decode is occurring, the state machine is using frames from its queue and we risk running low enough to hit the skip-to-keyframe logic.

If we know that no reordering can happen past the keyframe, then we can immediately drain the reorder queue and make those frames available to the state machine.

This should reduce the chances of us running low on decoded frames while waiting for a keyframe decode.

> 
> This will also output of order frames under some conditions. Try with the
> following patch (that assert if we output out of order frames)

I can't reproduce any assertions with this patch and your test video.
Blocks: 1113924
Priority: P1 → P2
(Reporter)

Comment 11

3 years ago
I can't reproduce it any longer... now playback just stutter.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → WORKSFORME
Flags: needinfo?(mozillamarcia.knous)
You need to log in before you can comment on or make changes to this bug.