Mark keySystemAccess as default to null and optional and robustness properties as no longer defaulting.

[mounirlamouri]

This is solving two issues:

• required for an object seems to not allow null, at least in Chrome bindings. It occurs to me that defaulting to null is doing what we were trying to achieve and works better with the bindings. I will assume that Chrome bindings are following the spec here. I did not double-check.
• Setting a default value for robustness means that in the steps where we compare with video and audio being present, we actually are always doing the check because per WebIDL, a property with a default value is always present. I had to put in prose the default value.

---

Preview | Diff

---

Preview | Diff

[chcunningham]

For robustness, its a little weird to have some fields in the dictionary with defaults and others without due to this validity nuance. What if we patch the validity algo to say:

If keySystemConfiguration.audioRubstness is not the empty string (""), audio MUST also be present.

Also, I misspelled audioRubstness and videoRubstness in the spec.

[mounirlamouri]

I think the only edge case where your solution wouldn't catch an issue is if someone does set audioRobustness: '' but doesn't set an audio configuration. In my proposal, we would reject but with your solution we wouldn't. The difference is that in EME you set the contentType and the robustness in the same object for video and audio separately so there is no need to guarantee consistency.

Edit: we can also purposefully keep this edge case not covered but I'm not sure it's worth doing just for the sake of a cleaner IDL.

[chcunningham]

I think the only edge case where your solution wouldn't catch an issue is if someone does set audioRobustness: '' but doesn't set an audio configuration.

Maybe thats a good thing? If empty is effectively the default (via algo), and if empty means "no robustness requirements", then having someone explicitly set this field to empty doesn't have to be an error if audio isn't present. Its a bit weird for a user to do this, but it doesn't break us or make the input ambiguous so I don't mind.

I think both IDL and algo are simpler this route.

[mounirlamouri]

Just to clarify, the case I have in mind is:

{
  video: {
    contentType: 'video/webm;codecs=vp9',
  },
  keySystemConfiguration: {
    keySystem: 'org.w3.clearkey',
    audioRobustness: '',
  }
}

which would translate in EME to something like:

{
  audioCapabilities: [
    { robustness: '' },
  ],
  videoCapabilities: [
    { contentType: 'video/webm; codecs="vp9"' },
  ],
}

and would therefore not be valid.

We would need to at least add logic to handle this case.

[chcunningham]

What do you think of doing the same thing to initDataType. It suffers the same issue of always being present.

[mounirlamouri]

The problem is different: for audio and video robustness, we have logic that says that if you set one or the other, there must be a configuration. Issue is that if you create a MediaCapabilitiesKeySystemConfiguration, per spec, it will set these two properties and always fail the check in https://w3c.github.io/media-capabilities/#mediaconfiguration

[chcunningham]

I think this is resolved by #138 (the nested audio/video =dictionary is optional and non-defaulted)

[chcunningham]

After sitting on this a bit more I think the approach is fine.

We might simplify what you have here even further with something like:
"if audioRobustness is present, set EME robustness attribute to config.keySystemConfig.audioRobustness"

And simply do nothing if it audioRobustness is not present. The default value from the EME config is "", so doing nothing should cause that default to take hold.

Maybe thats too subtle? Open to either route.

[mounirlamouri]

I don't have a strong opinion. In general, I find having clear defaults better than relying on side effects but if you feel strongly, I'm happy to update the PR.

[chcunningham]

"be a string DOMString"

Do you mean to say 'string' twice - just DOMString?

Set the robustness attribute to robustness.

How about this change in wording/style?

If config.keySystemConfiguration.videoRobustness is present, set robustness to config.keySystemConfiguration.videoRobustness. Otherwise, set robustness to "".

[mounirlamouri]

I fixed "string DOMString".

For the wording, the way I see it is that I prefer:

let robustness = "";
if (config.keySystemConfiguration.videoRobustness)
  robustness = config.keySystemConfiguration.videoRobustness;

over:

let robustness = config.keySystemConfiguration.videoRobustness
                               ? config.keySystemConfiguration.videoRobustness
                               : "";

(reason is that I prefer to have the default clear if there is one)

It's pretty much the same at the end so if you think it's clearer, I'm happy to apply the change.

[chcunningham]

I think with the recent change to nest robustness under a track config, the default of "" is no longer at odds with present (you check for the parent).

But the null key system access still makes sense.

[markafoltz]

I rebased this PR, but it seems to have been based on an earlier version of the spec, so it will need more updates before it can land.

[markafoltz]

The prose now should make sense with the current spec. I also incorporated the change from #223 since that was discussed as part of this PR.

I am not convinced that the spec steps need to be explicit about the default for robustness though. It is defined as a non-nullable DOMString in the IDL, it already has a default of "", and the corresponding EME attribute has a default of "".

Also note that the situation #107 (comment) (an audio robustness property without an audio KeySystemTrackConfiguration) is no longer possible.

It may be enough to check that it exists in the dictionary before copying the value over.

[markafoltz]

@xhwang-chromium If you have time to take a look it would be appreciated, as this deals with the EME integration part of the spec.

[xhwang-chromium]

In the IDL, robustness is already default to empty:

dictionary KeySystemTrackConfiguration {
  DOMString robustness = "";
  DOMString? encryptionScheme = null;
};

I it seems to me we don't need to have an explicit step to assign empty string to robustness if it's not specified. This is the same as how robustness in EME is handled.

[markafoltz]

Updated per feedback from @xhwang-chromium to remove the robustness default.