Closed Bug 1257727 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: cpearce, Unassigned)

References

Details

Attachments

(1 file)

We need to surface the WebM EME initData to HTML. This is probably a simple as implementing WebMDemuxer::GetCrypto().
Priority: -- → P2
- 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/
Attachment #8759004 - Flags: review?(jyavenard)
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) { }
Attachment #8759004 - Flags: review?(jyavenard) → review+
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 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/
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/40371bdc8088
1257729 - Update WebM handling to deal with encrypted WebMs. r=jya
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/40371bdc8088
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.