1345403 - Mark a video element as taint and never suspend it again if it is passed into drawImage()

Status: RESOLVED FIXED

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

Description

[Tzuhao Kuo [:kaku]]

Spawn from bug 1295921 comment 208.

Comment 1

[Tzuhao Kuo [:kaku]]

This bug is used to land patches 1~6 of Bug 129592. However, I have applied some modifications, and the original patches are old, so will ask for a review again!

Comment 2

[Tzuhao Kuo [:kaku]]

(In reply to Tzuhao Kuo [:kaku] from comment #1)
> This bug is used to land patches 1~6 of Bug 129592. However, I have applied
> some modifications, and the original patches are old, so will ask for a
> review again!

Sorry, it should be Bug 1295921, not Bug 129592.

Comment 3

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1345403 part 1 - Track decoder tainting;

Some uses of media elements should 'taint' the element so that the video doesn't participate in video decode suspending.
Add the infrastructure to track the taint status on MediaDecoder and mirror the status to MediaDecoderStateMachine.

Review commit: https://reviewboard.mozilla.org/r/118408/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118408/

Comment 4

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

Mark video element as tainted (stored on the decoder owned by video element) when the video is used as source to drawImage() on canvas.

Review commit: https://reviewboard.mozilla.org/r/118410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118410/

Comment 5

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

Review commit: https://reviewboard.mozilla.org/r/118412/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118412/

Comment 6

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1345403 part 4 - Clean up suspend timer canceling;

Review commit: https://reviewboard.mozilla.org/r/118414/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118414/

Comment 7

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1345403 part 5 - Test video suspend canceling;

Review commit: https://reviewboard.mozilla.org/r/118416/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/118416/

Comment 8

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=670e13dc43e78789b333225c15f52518b72a72c8
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7603ed833738a97e1a00a38ea9c1beee086488c2

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845217 [details]
Bug 1345403 part 1 - Track decoder tainting;

https://reviewboard.mozilla.org/r/118408/#review120338

Comment 10

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845218 [details]
Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

https://reviewboard.mozilla.org/r/118410/#review120344

::: dom/html/HTMLMediaElement.cpp:4761
(Diff revision 1)
>    }
>  
>    AddMediaElementToURITable();
>  
> +  // Notify the decoder of suspend taint.
> +  mDecoder->SetSuspendTaint(mHasSuspendTaint);

Move this just below |mDecoder->SetPlaybackRate(mPlaybackRate)| to group code setting MediaDecoder attributes.

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

https://reviewboard.mozilla.org/r/118412/#review120346

Comment 12

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

https://reviewboard.mozilla.org/r/118412/#review120348

::: dom/webidl/HTMLMediaElement.webidl:217
(Diff revision 1)
>    [Throws, Pref="media.seekToNextFrame.enabled"]
>    Promise<void> seekToNextFrame();
>  };
>  
>  /*
> - * This is an API for simulating visibility changes to help debug and write
> + * This APIs are used to help debug and write tests about suspend-video-decoding.

Use singular form because this 'is' an API which has multiple functions.

Comment 13

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845220 [details]
Bug 1345403 part 4 - Clean up suspend timer canceling;

https://reviewboard.mozilla.org/r/118414/#review120352

::: dom/media/MediaDecoderStateMachine.h:126
(Diff revision 1)
>    PlaybackEnded,
>    SeekStarted,
>    Invalidate,
>    EnterVideoSuspend,
> -  ExitVideoSuspend
> +  ExitVideoSuspend,
> +  BeginVideoSuspend,

The names are confusing. How about "StartVideoSuspendTimer" and "CancelVideoSuspendTimer"?

Comment 14

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8845221 [details]
Bug 1345403 part 5 - Test video suspend canceling;

https://reviewboard.mozilla.org/r/118416/#review120356

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

https://reviewboard.mozilla.org/r/118412/#review120494

r+ for the .webidl

Comment 16

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8845218 [details]
Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

https://reviewboard.mozilla.org/r/118410/#review120658

Comment 17

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845217 [details]
Bug 1345403 part 1 - Track decoder tainting;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118408/diff/1-2/

Comment 18

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845218 [details]
Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118410/diff/1-2/

Comment 19

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118412/diff/1-2/

Comment 20

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845220 [details]
Bug 1345403 part 4 - Clean up suspend timer canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118414/diff/1-2/

Comment 21

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845221 [details]
Bug 1345403 part 5 - Test video suspend canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118416/diff/1-2/

Comment 22

[Tzuhao Kuo [:kaku]]

Rebased and try again,
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0443f3e3da29276433ffd0403ab7d2008595657d
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1efb20954950d758176c56d47c17f61f82bff2cc

Comment 23

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845217 [details]
Bug 1345403 part 1 - Track decoder tainting;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118408/diff/2-3/

Comment 24

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845218 [details]
Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118410/diff/2-3/

Comment 25

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118412/diff/2-3/

Comment 26

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845220 [details]
Bug 1345403 part 4 - Clean up suspend timer canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118414/diff/2-3/

Comment 27

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845221 [details]
Bug 1345403 part 5 - Test video suspend canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118416/diff/2-3/

Comment 28

[Tzuhao Kuo [:kaku]]

The try server was weird yesterday, reabsed and try again.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=25a90945da489ef269852749f2441f522510a3c2
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca30d0e6e9b3766fb6cd148c06c2b11ee62ed000

Comment 29

[Tzuhao Kuo [:kaku]]

Try looks very good, thanks for the review!

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7a7d6e9b8329
part 1 - Track decoder tainting; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/ba072186c917
part 2 - Mark element tainted when DrawImage is used; r=jwwang,mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/a3e4ce12194f
part 3 - Test element becomes tainted by DrawImage; r=jwwang,smaug
https://hg.mozilla.org/integration/autoland/rev/ac12e246a808
part 4 - Clean up suspend timer canceling; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/f6415ae0ba89
part 5 - Test video suspend canceling; r=jwwang

Comment 31

[Bob Clary [:bc] (inactive)]

kaku: This caused perma failures in Autophone's Mdm tests:

https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=f6415ae0ba892b17b807c2dd1e0961140b1b80d0&filter-searchStr=autophone

Can you back this out and fix the issues or the tests before relanding?

You can test this on try with

try: -b do -p android-api-15 -u autophone-mochitest-media -t none --artifact

Comment 32

[Iris Hsiao [:ihsiao]]

sorry had to back this out for permanent failure in autophone Mdm tests like https://treeherder.mozilla.org/logviewer.html#?job_id=83287626&repo=autoland&lineNumber=808

https://hg.mozilla.org/integration/autoland/rev/135488a851a5

Comment 33

[Tzuhao Kuo [:kaku]]

I will then disable those newly add tests on Android and open bug 1346705 for tracking/fixing them.

Comment 34

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845217 [details]
Bug 1345403 part 1 - Track decoder tainting;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118408/diff/3-4/

Comment 35

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845218 [details]
Bug 1345403 part 2 - Mark element tainted when DrawImage is used;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118410/diff/3-4/

Comment 36

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845219 [details]
Bug 1345403 part 3 - Test element becomes tainted by DrawImage;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118412/diff/3-4/

Comment 37

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845220 [details]
Bug 1345403 part 4 - Clean up suspend timer canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118414/diff/3-4/

Comment 38

[Tzuhao Kuo [:kaku]]

Comment on attachment 8845221 [details]
Bug 1345403 part 5 - Test video suspend canceling;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/118416/diff/3-4/

Comment 39

[Tzuhao Kuo [:kaku]]

Try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d58ae40ec66d357b9038019b4b0ee541f51cbc1
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ce13cf27550eabf5e753332ce7e11ac5ed082df

Comment 40

[Tzuhao Kuo [:kaku]]

Try looks good, thanks for review and check in again!

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2305bfbe6545
part 1 - Track decoder tainting; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/99e3488b1ea4
part 2 - Mark element tainted when DrawImage is used; r=jwwang,mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/7f9def21c5b7
part 3 - Test element becomes tainted by DrawImage; r=jwwang,smaug
https://hg.mozilla.org/integration/autoland/rev/850a10a1e44f
part 4 - Clean up suspend timer canceling; r=jwwang
https://hg.mozilla.org/integration/autoland/rev/be9cfb690427
part 5 - Test video suspend canceling; r=jwwang

Comment 42

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2305bfbe6545
https://hg.mozilla.org/mozilla-central/rev/99e3488b1ea4
https://hg.mozilla.org/mozilla-central/rev/7f9def21c5b7
https://hg.mozilla.org/mozilla-central/rev/850a10a1e44f
https://hg.mozilla.org/mozilla-central/rev/be9cfb690427