1217226 - Intel VP9 decoder playback is pear shaped

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Steps to reproduce:

* Enable Intel VP9 decoder on supported hardware
* Navigate to link
* Observe video shape

Expected results:

Video should be 16x9

Actual results:

Video is 4x3

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: P1. Use VideoInfo data rather than attempt to detect value in stream

WIP

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=005360063bd4

Comment 3

[Jean-Yves Avenard [:jya]]

Attachment: P2. Implement WMFMediaDataDecoder::ConfigurationChanged

Implement WMF::ConfigurationChanged to be notified when the VideoInfo object has changed.

Comment 4

[Jean-Yves Avenard [:jya]]

Attachment: P1. Use VideoInfo data rather than attempt to detect value in stream

Use VideoInfo provided by the reader. The H264Converter ensure that the dimensions provided in the VideoInfo are valid by decoding the SPS which is exactly what the WMF decoder would do. We can as such fully rely on the VideoInfo object to detect the dimension of the decoded picture and its display sizes

Comment 5

[Jean-Yves Avenard [:jya]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5733ed90de6d

Comment 6

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

Comment on attachment 8677859 [details] [diff] [review]
P2. Implement WMFMediaDataDecoder::ConfigurationChanged

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

::: dom/media/platforms/wmf/WMFMediaDataDecoder.h
@@ +104,5 @@
>    void ProcessDrain();
>  
>    void ProcessShutdown();
>  
> +  // Called on the task queue. Tell the MFT that the next Input will have a 

Trailing space.

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: P1. Use VideoInfo display size data rather than attempt to detect value in stream

Reworked to remove doubts. We keep using the WMF dimension for the picture itself, the display dimensions however are taken from the VideoInfo. That way we are certain there will never be a discrepency between the allocated DXVA surface and the actual decoded size.

Comment 8

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to Jean-Yves Avenard [:jya] from comment #5)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=5733ed90de6d

Looks like this build job failed.

Comment 9

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

Comment on attachment 8678668 [details] [diff] [review]
P1. Use VideoInfo display size data rather than attempt to detect value in stream

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

FYI, the thing that had me most worried about your v1 patch was the fact that you removed the IsValidVideoRegion() call.

Comment 10

[Jean-Yves Avenard [:jya]]

(In reply to Chris Pearce (:cpearce) from comment #9)
> Comment on attachment 8678668 [details] [diff] [review]
> P1. Use VideoInfo display size data rather than attempt to detect value in
> stream
> 
> Review of attachment 8678668 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> FYI, the thing that had me most worried about your v1 patch was the fact
> that you removed the IsValidVideoRegion() call.

Ah..
The SPS decoder performs similar check on the dimensions (so that it can't overflow), so IsValidVideoRegion can never return false on the dimensions found in the VideoInfo.

In any case, it's probably better that way.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8415ac131a72
https://hg.mozilla.org/integration/mozilla-inbound/rev/cacab6d4b6e7

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8415ac131a72
https://hg.mozilla.org/mozilla-central/rev/cacab6d4b6e7