Closed Bug 1217226 Opened 4 years ago Closed 4 years ago

Intel VP9 decoder playback is pear shaped

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: ajones, Assigned: jya)

References

()

Details

Attachments

(2 files, 2 obsolete files)

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
Assignee: nobody → jyavenard
Blocks: 1213131
No longer blocks: 1213498
Blocks: 1101885
No longer blocks: 1213131
Depends on: 1195094
Implement WMF::ConfigurationChanged to be notified when the VideoInfo object has changed.
Attachment #8677859 - Flags: review?(cpearce)
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
Attachment #8677861 - Flags: review?(cpearce)
Attachment #8677778 - Attachment is obsolete: true
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.
Attachment #8677859 - Flags: review?(cpearce) → review+
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.
Attachment #8678668 - Flags: review?(cpearce)
Attachment #8677861 - Attachment is obsolete: true
Attachment #8677861 - Flags: review?(cpearce)
Priority: -- → P1
(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 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.
Attachment #8678668 - Flags: review?(cpearce) → review+
(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.
https://hg.mozilla.org/mozilla-central/rev/8415ac131a72
https://hg.mozilla.org/mozilla-central/rev/cacab6d4b6e7
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Depends on: 1222103
Depends on: 1222201
You need to log in before you can comment on or make changes to this bug.