823646 - Certain H.264 movie are not shown properly with media.windows-media-foundation.enabled = true

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Alice0775 White]

Attachment: screenshot

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/21195f52311c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20121220 Firefox/20.0 ID:20121220030912

See report http://forums.mozillazine.org/viewtopic.php?p=12557309#p12557309 .

Certain h.264 movie are not show properly. sound is ok.
 - http://ie.microsoft.com/testdrive/Graphics/VideoFormatSupport/Default.html
 - http://devfiles.myopera.com/articles/4791/0.htm
 - http://easyhtml5video.com

This problem does not happen in Windows8.

Steps to reproduce:
1. Make sure media.windows-media-foundation.enabled = true
2. Open http://ie.microsoft.com/testdrive/Graphics/VideoFormatSupport/Default.html
3. Play back if necessary

Actual results:
Diagonal stripe pattern is displayed

Comment 2

[Gingerbread Man]

An example of an H.264 video that works:
http://www.quirksmode.org/html5/tests/video.html

Comment 4

[(not currently active) Ted Mielczarek]

Example URL from my dupe: http://graphics8.nytimes.com/packages/video/multimedia/bundles/projects/2012/AvalancheDeploy/airbag_final.mp4

and from that bug:
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> FWIW:
> 
> $ ffprobe ~/airbag_final.mp4 
> avprobe version 0.8.4-4:0.8.4-0ubuntu0.12.04.1, Copyright (c) 2007-2012 the
> Libav developers
>   built on Nov  6 2012 16:51:33 with gcc 4.6.3
> Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/home/luser/airbag_final.mp4':
>   Metadata:
>     major_brand     : mp42
>     minor_version   : 0
>     compatible_brands: mp42mp41
>     creation_time   : 2012-12-10 23:34:14
>   Duration: 00:00:05.00, start: 0.033367, bitrate: 1288 kb/s
>     Stream #0.0(eng): Video: h264 (Main), yuv420p, 200x312 [PAR 1:1 DAR
> 25:39], 1277 kb/s, 29.97 fps, 29.97 tbr, 29970 tbn, 59.94 tbc
>     Metadata:
>       creation_time   : 2012-12-10 23:34:14

Comment 5

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

The problem is that on Windows 7 the video stream media type changes, in particular the frame size changes, which messes up our calculation of where the YCbCr planes are in the output video frames. I don't know why, maybe WMF is getting the frame size from metadata, and then the actual frame size is different.

Anyway, the solution is to check the |flags| returned by IMFSourceReader for MF_SOURCE_READERF_CURRENTMEDIATYPECHANGED and if so reconfigure the our reader to handle the new video frame geometry.

I'll also add a check for this on the audio decode, but I won't handle it. I'm not sure if media type changes happen in audio in WMF in practice.

Comment 6

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

Attachment: Patch v1

* Detect video media type changes, and reconfigure the reader to handle new video frame geometries.
* Stop decoding audio upon audio media type changes. If we encounter audio media type changes in the wild we can fix it properly; I'm not sure if it happens in actual media or not, but I think it unlikely.
* Handle "pan and scan", "minimum aperture", and "geometric aperture", this allows the media to specify a region of the video frame as being displayed. I don't think we'll encounter this in MP4, but we should handle it just in case.
* Make some line endings consistent in WMDecoder.cpp.
* Remove WMFReader::mVideoWidth, it's not necessary to store it.

Comment 7

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

Comment on attachment 703132 [details] [diff] [review]
Patch v1

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

::: content/media/wmf/WMFReader.cpp
@@ +296,5 @@
> +
> +HRESULT
> +WMFReader::ConfigureVideoFrameGeometry(IMFMediaType* aMediaType)
> +{
> +  NS_ENSURE_ARG(aMediaType);

This needs to be:

NS_ENSURE_TRUE(aMediaType != nullptr, E_POINTER);

As otherwise I get a type mismatch error in VS2012 and porbably also in new versions of GCC. (The error is because NS_ENSURE_ARG returns an nsresult, but this function returns an HRESULT).

Comment 8

[Paul Adenot (:padenot)]

Comment on attachment 703132 [details] [diff] [review]
Patch v1

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

It would be cool to have a reftest for that. Actually, it would be cool to have reftests for h264, we only have theora and webm. I filed bug 832296 for this.

::: content/media/wmf/WMFReader.cpp
@@ +253,5 @@
> +  MFVideoArea videoArea;
> +  memset(&videoArea, 0, sizeof(MFVideoArea));
> +  if (panScan) {
> +    hr = aMediaType->GetBlob(MF_MT_PAN_SCAN_APERTURE,
> +                             (UINT8*)&videoArea, 

nit: trailing whitespace, and others below that.

Comment 9

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d58243e4cd6

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/3d58243e4cd6