Bug 212568

Summary: Remove ENABLE_VIDEO_TRACK ifdef guards
Product: WebKit Reporter: Philippe Normand <pnormand>
Component: MediaAssignee: Philippe Normand <pnormand>
Status: RESOLVED FIXED    
Severity: Normal CC: allan.jensen, annulen, benjamin, calvaris, cdumez, cgarcia, changseok, cmarcelo, darin, dino, eric.carlson, esprehn+autocc, ews-watchlist, glenn, graouts, gustavo, gyuyoung.kim, hta, japhet, jbedard, jer.noble, kangil.han, keith_miller, kondapallykalyan, macpherson, mark.lam, menard, mkwst, msaboff, pdr, philipj, ryuan.choi, saam, sergio, tommyw, tzagallo, vjaquez, youennf
Priority: P2    
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
WIP Patch
none
Patch
youennf: review+
patch for landing none

Description Philippe Normand 2020-05-30 09:40:33 PDT 
This feature seems to be enabled on all ports nowadays, and the spec hasn't changed. Can we unconditionally enable this feature?
Comment 1 Eric Carlson 2020-06-01 06:29:04 PDT 
I think we can
Comment 2 Philippe Normand 2020-06-03 11:21:00 PDT 
I've started a patch, going to be big.
Comment 3 Philippe Normand 2020-06-04 10:34:53 PDT 
While working on this I'm checking the build with ENABLE(VIDEO) disabled. This hasn't be tested in a looonng time. What should we do with the GPU process when VIDEO is disabled?
Comment 4 Philippe Normand 2020-06-04 10:41:23 PDT 
Created attachment 401039 [details]
WIP Patch
Comment 5 Philippe Normand 2020-06-04 10:55:26 PDT 
Created attachment 401044 [details]
WIP Patch
Comment 6 Philippe Normand 2020-06-05 04:03:00 PDT 
Created attachment 401140 [details]
WIP Patch
Comment 7 Philippe Normand 2020-06-06 02:05:09 PDT 
Created attachment 401243 [details]
WIP Patch
Comment 8 Philippe Normand 2020-06-06 03:19:12 PDT 
Created attachment 401246 [details]
WIP Patch
Comment 9 Philippe Normand 2020-06-06 03:50:40 PDT 
Created attachment 401247 [details]
WIP Patch
Comment 10 Philippe Normand 2020-06-06 04:11:42 PDT 
Created attachment 401248 [details]
WIP Patch
Comment 11 Philippe Normand 2020-06-06 04:28:12 PDT 
Created attachment 401250 [details]
WIP Patch
Comment 12 Philippe Normand 2020-06-06 04:48:30 PDT 
Created attachment 401251 [details]
WIP Patch
Comment 13 Philippe Normand 2020-06-06 06:55:37 PDT 
Created attachment 401255 [details]
Patch
Comment 14 youenn fablet 2020-06-06 11:06:14 PDT 
Comment on attachment 401255 [details]
Patch

Nice!

Have you tried compiling with ENABLE_VIDEO disabled?

View in context: https://bugs.webkit.org/attachment.cgi?id=401255&action=review

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:3986
> +    BE88E0D91715D2A200658D98 /* AudioTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = BE88E0CD1715D2A200658D98 /* AudioTrack.h */; settings = {ATTRIBUTES = (Private, ); }; };

Seems strange, was it done by hand?
Comment 15 Philippe Normand 2020-06-06 11:20:40 PDT 
(In reply to youenn fablet from comment #14)
> Comment on attachment 401255 [details]
> Patch
> 
> Nice!
> 
> Have you tried compiling with ENABLE_VIDEO disabled?
> 

Yes, it's broken. I'm not sure how to deal with the GPU process in this case.

> View in context:
> https://bugs.webkit.org/attachment.cgi?id=401255&action=review
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:3986
> > +    BE88E0D91715D2A200658D98 /* AudioTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = BE88E0CD1715D2A200658D98 /* AudioTrack.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 
> Seems strange, was it done by hand?

Yes :)
Comment 16 Philippe Normand 2020-06-06 11:21:25 PDT 
(In reply to Philippe Normand from comment #15)
> (In reply to youenn fablet from comment #14)
> > Comment on attachment 401255 [details]
> > Patch
> > 
> > Nice!
> > 
> > Have you tried compiling with ENABLE_VIDEO disabled?
> > 
> 
> Yes, it's broken. I'm not sure how to deal with the GPU process in this case.
> 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=401255&action=review
> > 
> > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:3986
> > > +    BE88E0D91715D2A200658D98 /* AudioTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = BE88E0CD1715D2A200658D98 /* AudioTrack.h */; settings = {ATTRIBUTES = (Private, ); }; };
> > 
> > Seems strange, was it done by hand?
> 
> Yes :)

I'll check this again, seems suspicious indeed.
Comment 17 Philippe Normand 2020-06-06 11:22:30 PDT 
Comment on attachment 401255 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401255&action=review

>>>> Source/WebCore/WebCore.xcodeproj/project.pbxproj:3986
>>>> +    BE88E0D91715D2A200658D98 /* AudioTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = BE88E0CD1715D2A200658D98 /* AudioTrack.h */; settings = {ATTRIBUTES = (Private, ); }; };
>>> 
>>> Seems strange, was it done by hand?
>> 
>> Yes :)
> 
> I'll check this again, seems suspicious indeed.

Ah it's only a white-space issue. I'll fix this before landing.
Comment 18 youenn fablet 2020-06-06 11:28:41 PDT 
(In reply to Philippe Normand from comment #15)
> (In reply to youenn fablet from comment #14)
> > Comment on attachment 401255 [details]
> > Patch
> > 
> > Nice!
> > 
> > Have you tried compiling with ENABLE_VIDEO disabled?
> > 
> 
> Yes, it's broken. I'm not sure how to deal with the GPU process in this case.

I don't think ENABLE_VIDEO disabled and GPU process enabled is that important.
But it might be worth keeping ENABLE_VIDEO disabled and GPU Process disabled.
Comment 19 Philippe Normand 2020-06-07 01:32:12 PDT 
Created attachment 401288 [details]
patch for landing
Comment 20 Philippe Normand 2020-06-07 02:56:49 PDT 
Fixed in r262695