Bug 189238

Summary: [GStreamer][EME][webm] Add the WebM encrypted content support in matroskademux
Product: WebKit Reporter: Yacine Bandou <bandou.yacine>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboya, calvaris, commit-queue, cturner, olivier.blin, pnormand, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 190253    
Bug Blocks: 189239, 189240    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Yacine Bandou 2018-09-03 02:37:59 PDT 
Modifications on GstMatroskaDemux tracked in GStreamer bugzilla:
https://bugzilla.gnome.org/show_bug.cgi?id=765275
https://bugzilla.gnome.org/attachment.cgi?id=365211
Comment 1 Yacine Bandou 2018-09-03 02:57:29 PDT 
Created attachment 348764 [details]
Patch
Comment 2 Xabier Rodríguez Calvar 2018-09-03 06:27:39 PDT 
Looks good, I'll let Charlie have a look as well and we can land this here when it does upstream or it is needed for something else.
Comment 3 Yacine Bandou 2018-10-03 11:32:00 PDT 
Created attachment 351538 [details]
Patch
Comment 4 Xabier Rodríguez Calvar 2018-10-05 06:07:35 PDT 
Comment on attachment 351538 [details]
Patch

We need changes of bug 190253 as well.
Comment 5 Yacine Bandou 2018-10-05 06:38:34 PDT 
Created attachment 351670 [details]
Patch
Comment 6 WebKit Commit Bot 2018-10-07 23:18:24 PDT 
Comment on attachment 351670 [details]
Patch

Clearing flags on attachment: 351670

Committed r236911: <https://trac.webkit.org/changeset/236911>
Comment 7 WebKit Commit Bot 2018-10-07 23:18:25 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-10-07 23:19:34 PDT 
<rdar://problem/45080591>
Comment 9 Philippe Normand 2018-10-11 08:22:41 PDT 
Why are those patches needed? I'd rather flag related layout tests than adding them, to be honest.

The use of GST_PROTECTION_UNSPECIFIED_SYSTEM_ID without wrapping ifdef broke the build BTW.
Comment 10 Philippe Normand 2018-10-11 08:39:14 PDT 
https://bugs.webkit.org/show_bug.cgi?id=190471
Comment 11 Yacine Bandou 2018-10-11 09:08:17 PDT 
(In reply to Philippe Normand from comment #9)
> Why are those patches needed? I'd rather flag related layout tests than
> adding them, to be honest.

They are needed for encrypted WebM.

media/encrypted-media/clearKey/clearKey-encrypted-webm-event-mse.html
media/encrypted-media/clearKey/clearKey-webm-video-playback-mse.html

> 
> The use of GST_PROTECTION_UNSPECIFIED_SYSTEM_ID without wrapping ifdef broke
> the build BTW.

I don't understand how it broke the build, if you have updated your jhbuild.
Comment 12 Philippe Normand 2018-10-11 09:14:11 PDT 
(In reply to Yacine Bandou from comment #11)
> (In reply to Philippe Normand from comment #9)
> > Why are those patches needed? I'd rather flag related layout tests than
> > adding them, to be honest.
> 
> They are needed for encrypted WebM.
> 
> media/encrypted-media/clearKey/clearKey-encrypted-webm-event-mse.html
> media/encrypted-media/clearKey/clearKey-webm-video-playback-mse.html
> 

Can't we flag/skip these two until gst 1.16 is released? By the end of the year hopefully.

> > 
> > The use of GST_PROTECTION_UNSPECIFIED_SYSTEM_ID without wrapping ifdef broke
> > the build BTW.
> 
> I don't understand how it broke the build, if you have updated your jhbuild.

Because I didn't use jhbuild. Whenever possible we should try to avoid frankensteining our jhbuild with patches, especially those adding new API.
Comment 13 Yacine Bandou 2018-10-11 09:24:13 PDT 
(In reply to Philippe Normand from comment #12)
> (In reply to Yacine Bandou from comment #11)
> > (In reply to Philippe Normand from comment #9)
> > 
> > I don't understand how it broke the build, if you have updated your jhbuild.
> 
> Because I didn't use jhbuild. Whenever possible we should try to avoid
> frankensteining our jhbuild with patches, especially those adding new API.

Understood.