1257727 - [EME] Surface WebM initData to HTML; implement WebMDemuxer::GetCrypto()

Status: RESOLVED FIXED

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

Description

[Chris Pearce [:cpearce (Not reading bugmail)]]

We need to surface the WebM EME initData to HTML. This is probably a simple as implementing WebMDemuxer::GetCrypto().

Comment 1

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Attachment: MozReview Request: Bug 1257727, 1257729 - Update WebM handling to deal with encrypted WebMs r?jya

- WebMDemuxer will read crypto information from WebM metadata.
- WebMDemumer adds crypto information to samples.
- WebMDemuxer can now return encryption info from GetCrypto().
- WebMDexmuer will not attempt to peek encrypted frames as it
will give back garbage data. This means resolution changes
internal to encrypted WebM files will not work.
- WebMDecoder now exposes a single string version of
CanHandleMediaType. This is done in the same way as the
Mp4Decoder, so that the future update to MediaKeySystemAccess
for WebM handling can maintain the same conventions.

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

Comment 2

[Jean-Yves Avenard [:jya]]

Comment on attachment 8759004 [details]
MozReview Request: Bug 1257727, 1257729 - Update WebM handling to deal with encrypted WebMs r?jya

https://reviewboard.mozilla.org/r/57108/#review53832

LGTM from a demuxer point of view. I don't know enough about the crypto side of things to confirm one way or another.

::: dom/media/webm/WebMDemuxer.cpp:514
(Diff revision 1)
>  UniquePtr<EncryptionInfo>
>  WebMDemuxer::GetCrypto()
>  {
> +  unsigned int nVideoTracks = 0;
> +  unsigned int nAudioTracks = 0;
> +  bool hasEncryptedTrack = false;

don't need this; EncryptionInfo::IsEncrypted() will return true only if AddInitData was called on it.

::: dom/media/webm/WebMDemuxer.cpp:516
(Diff revision 1)
>  {
> +  unsigned int nVideoTracks = 0;
> +  unsigned int nAudioTracks = 0;
> +  bool hasEncryptedTrack = false;
> +
> +  int r = nestegg_track_count(Context(TrackInfo::kVideoTrack), &nVideoTracks);

We're doing this in ReadMetadata already, can't we somehow pre-calculate this in ReadMetadatada instead?

::: dom/media/webm/WebMDemuxer.cpp:531
(Diff revision 1)
> +    WEBM_DEBUG("nestegg_track_count on audio context failed r=%d", r);
> +    return nullptr;
> +  }
> +
> +  auto crypto = MakeUnique<EncryptionInfo>();
> +  for (unsigned int track = 0; track < nVideoTracks; ++track) {

personally, I don't like pre-increment, but seeing that's what's already in use.

::: dom/media/webm/WebMDemuxer.cpp:551
(Diff revision 1)
> +      hasEncryptedTrack = true;
> +      crypto->AddInitData(NS_LITERAL_STRING("webm"), Move(trackCrypto.mKeyId));
> +    }
> +  }
> +
> +  if (hasEncryptedTrack) {

return crypto->IsEncrypted() ? crypto : nullptr;

::: dom/media/webm/WebMDemuxer.cpp:1096
(Diff revision 1)
>  }
>  
>  void
>  WebMTrackDemuxer::UpdateSamples(nsTArray<RefPtr<MediaRawData>>& aSamples)
>  {
> +  for (size_t i = 0; i < aSamples.Length(); i++) {

could use for (const auto& sample : aSamples) { }

Comment 3

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://reviewboard.mozilla.org/r/57108/#review53832

> We're doing this in ReadMetadata already, can't we somehow pre-calculate this in ReadMetadatada instead?

I've added a member to store the crypto data, and populate it when we parse the metadata. GetCrypto should now return a UniquePtr to a copy of that memeber rather than regenerating the info every time.

> personally, I don't like pre-increment, but seeing that's what's already in use.

Removed these as part of the above change (though we still have the ones in ReadMetadata).

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Comment on attachment 8759004 [details]
MozReview Request: Bug 1257727, 1257729 - Update WebM handling to deal with encrypted WebMs r?jya

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

Comment 5

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=96855e301b10

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40371bdc8088
1257729 - Update WebM handling to deal with encrypted WebMs. r=jya

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40371bdc8088