1222201 - Video playback distortion, The right end of youtube video is stretched several pixels

Status: VERIFIED FIXED

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

Description

[Alice0775 White]

Attachment: screenshot, Nightly45.0a1 .VS. Beta43.0b1

[Tracking Requested - why for this release]: regression of Video playback distortion


Video playback distortion, The right end of youtube video is stretched several pixels

See screenshot

Steps To Reproduce:
1. Open video ( e.g. https://www.youtube.com/watch?v=KKTGiNqUKUQ )
2. Select 480P
3. Seek at around 00:10

Actual Results:
The right end of youtube video is stretched several pixels


Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=86d1b3a8aecb978a7e656e8c8018e227333d9ae5&tochange=b4401440ad83

Suspect: 8415ac131a72	Jean-Yves Avenard — Bug 1217226: P1. Use VideoInfo display size data rather than attempt to detect value in stream. r=cpearce

Comment 1

[Alice0775 White]

And https://www.youtube.com/watch?v=Z5jyEphbGo4 480P around 00:05

Comment 2

[Ralph Giles (:rillian) | needinfo me]

Probably something with the the frame crop metadata.

Comment 3

[Jean-Yves Avenard [:jya]]

From talking to roc, this is due to our dxva compositor not handling offsets. Which vp9 videos often uses

Comment 4

[arni2033]

@Ralph Giles:   The regression window is provided in comment 0. Btw, I got the same regression window

Comment 5

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

Milan - who fixes compositor bugs?

Comment 6

[Jean-Yves Avenard [:jya]]

I have a suspicion on what's going on with the dimensions returned by WMF. I'll look into it later today. Still need to implement margins support with dxva

Comment 7

[Jean-Yves Avenard [:jya]]

Attachment: Only use container calculated dimensions

Use calculated dimensions found in SPS rather than the values returned by the WMF.
Turned out that the attribute MF_MT_FRAME_SIZE is actually the stride size rather than the usable size and previously we would use the values returned by GetPictureRegion. I got incorrectly stirred by the name of the attribute and the name of the variables.

Comment 8

[Jean-Yves Avenard [:jya]]

Comment on attachment 8687833 [details] [diff] [review]
Only use container calculated dimensions

kind of forgot to do hg qnew and uploaded the big ffvpx one instead :)

Comment 9

[Jean-Yves Avenard [:jya]]

Attachment: Only use container calculated dimensions

Use calculated dimensions found in SPS rather than the values returned by the WMF.
Turned out that the attribute MF_MT_FRAME_SIZE is actually the stride size rather than the usable size and previously we would use the values returned by GetPictureRegion. I got incorrectly stirred by the name of the attribute and the name of the variables.

Comment 10

[Chris Pearce [:cpearce (GMT+13)]]

Comment on attachment 8687834 [details] [diff] [review]
Only use container calculated dimensions

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

Looks like we'll need a similar change to media/gmp-clearkey/0.1/WMFH264Decoder.cpp.

Comment 11

[Jean-Yves Avenard [:jya]]

(In reply to Chris Pearce (:cpearce) from comment #10)
> Comment on attachment 8687834 [details] [diff] [review]
> Only use container calculated dimensions
> 
> Review of attachment 8687834 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks like we'll need a similar change to
> media/gmp-clearkey/0.1/WMFH264Decoder.cpp.

no it's fine there.
It calls wmf::GetPictureRegion(aMediaType, pictureRegion); which does the proper thing (or at least get the right value).

The alternative approach would be to re-add the use of GetPictureRegion().. I'm in two mind about it, as it guarantees that there's no regression over the earlier code which has worked fine for a long time now

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/87b9a8debdad

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/87b9a8debdad

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracking as this is a high end-user impact regression.

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Jean-Yves, should we consider uplifting this to Aurora44? Thanks!

Comment 17

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Alice0775 White, could you please verify that this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 18

[Jean-Yves Avenard [:jya]]

Comment on attachment 8687834 [details] [diff] [review]
Only use container calculated dimensions

Approval Request Comment
[Feature/regressing bug #]: 1222201
[User impact if declined]: Visual corruptions in MP4 data on windows (obvious when using YouTube)
[Describe test coverage new/current, TreeHerder]: In central for 2 weeks.
[Risks and why]: Low, however the fix may not be sufficient and there may be a need to revert the entire set of changes in the WMF which were made to support the intel hardware decoder (now disabled by default)
[String/UUID change made/needed]: None

Comment 19

[Alice0775 White]

(In reply to Ritu Kothari (:ritu) from comment #17)
> Alice0775 White, could you please verify that this issue is fixed as
> expected on a latest Nightly build? Thanks!

I cannot reproduce the problem on latest Nightly45.0a1 anymore.

https://hg.mozilla.org/mozilla-central/rev/47b49b0d32360fab04b11ff9120970979c426911
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20151130030228

But, of course, I can still reproduce the problem on Aurora44.0a2.

https://hg.mozilla.org/releases/mozilla-aurora/rev/7ea5152b84175b42bfe7519e450d24e546e014db
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0 ID:20151129004009

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

(In reply to Alice0775 White from comment #19)
> (In reply to Ritu Kothari (:ritu) from comment #17)
> > Alice0775 White, could you please verify that this issue is fixed as
> > expected on a latest Nightly build? Thanks!
> 
> I cannot reproduce the problem on latest Nightly45.0a1 anymore.
> 
> https://hg.mozilla.org/mozilla-central/rev/
> 47b49b0d32360fab04b11ff9120970979c426911
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
> ID:20151130030228
> 
> But, of course, I can still reproduce the problem on Aurora44.0a2.
> 
> https://hg.mozilla.org/releases/mozilla-aurora/rev/
> 7ea5152b84175b42bfe7519e450d24e546e014db
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0
> ID:20151129004009

Thank you for doing the verification! We will uplift the fix to Aurora44.

Comment 21

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8687834 [details] [diff] [review]
Only use container calculated dimensions

This fix has been verified on Nightly, it makes sense to uplift to Aurora44.

Comment 22

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/fd69b780f492

Comment 23

[alex_mayorga]

¡Hola Jean-Yves!

Verified with steps from https://bugzilla.mozilla.org/show_bug.cgi?id=1222103#c1

This works on Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20151201030226 CSet: 66a6d7ec9534b9d7847b665142fef0dd87623768

¡Gracias!

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/fd69b780f492

Comment 25

[Ada [:adalucinet]]

Reproduced with Nightly from 2015-11-05 under Windows 7 64-bit.
Verified fixed with Firefox 44 beta 4 (Build ID: 20151228134903) and latest Developer Edition 45.0a2 (from 2016-01-03), under Windows 7 64-bit and Mac OS X 10.11.