1293963 - (Phase_1_Shutdown_Decoder) [Meta] Suspend-video-decoder: phase-1 shipping

Status: NEW

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

Description

[Tzuhao Kuo [:kaku]]

Per offline discussion, we are going to ship the phase 1 of suspend-video-decoder. In the phase 1, we target on the SAFE cases. Our assumption for SAFE cases is that for videos under certain resolution, the resuming time could be ignored.

Comment 1

[Tzuhao Kuo [:kaku]]

So, the question is "What exact is the target resolution? and is the target resolution platform-dependent?" 

Looks like that we don't have the telemetry for "resuming time v.s. resolution", we might this data to help deciding the target resolution.

Comment 2

[u480271]

In discussions, we've said to limit to "480" video or less. I took that to mean the default resolution of YouTube videos. The video ad I'm testing with is 720 × 406.

Comment 3

[Tzuhao Kuo [:kaku]]

Yes, we did and I will implement it with 480P for now. But at the same time, we can still collect telemetry data for more rigorous decision later. Thanks :gerald for implementing the telemetry.

Comment 4

[Tzuhao Kuo [:kaku]]

Since bug 1294656 has been landed, we can also enable the suspend-video-decoder feature for those video files without audio track, which could be resumed quickly by fast seek.

Comment 5

[Tzuhao Kuo [:kaku]]

Attachment: Bug 1293963 - phase-1 shipping;

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

Comment 6

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

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

Comment 7

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

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

https://reviewboard.mozilla.org/r/76580/#review74716

1. Extract all the policy code to a function so it is easier to maintain when we add more policies.
2. Do we need to switch the "shipping phase" back and forth? If not, there is no need to encode the concept in the code.
3. IIUC, VideoInfo::mDisplay is a scaled size and you should look at mImage. Please ask come gfx guys to confirm it.

Comment 8

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

https://reviewboard.mozilla.org/r/76580/#review74716

> 2. Do we need to switch the "shipping phase" back and forth? If not, there is no need to encode the concept in the code.

I think that we could turn the nightly channel into phase-0 again once the phase-1 code has been checked into aurora, so that we can collect more telemetry data. 
Alos, keeps this concept might help wrting test cases.

Comment 9

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

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

https://reviewboard.mozilla.org/r/76580/#review74716

It looks like a maintainace hazard to have different phases on difference release channels. It is also confusing when you have to check the phase in order to write a test case to run correctly on that phase.

Comment 10

[Tzuhao Kuo [:kaku]]

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

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

Comment 11

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

Comment on attachment 8788036 [details]
Bug 1293963 - phase-1 shipping;

https://reviewboard.mozilla.org/r/76580/#review74732

Comment 12

[Tzuhao Kuo [:kaku]]

Thanks for the review!

Try looks good at https://treeherder.mozilla.org/#/jobs?repo=try&revision=51d36fcddaf2

Comment 13

[Tzuhao Kuo [:kaku]]

I think we should NOT land this patch before bug 1284389 and bug 1295921 being resolved, right?

Comment 14

[u480271]

(In reply to Tzuhao Kuo [:kaku] from comment #13)
> I think we should NOT land this patch before bug 1284389 and bug 1295921
> being resolved, right?

Yeah, let's hang off until we can land bug 1295921. I'm working hard of getting head way into that bug.

Comment 15

[Tzuhao Kuo [:kaku]]

Thanks for taking the tough bug!

I will start a long PTO since next Wed. on (around two weeks, until 10/2), so once you finished bug 1295921, please feel free to take this one and land the patch if you want.

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.