Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More WebIDL Issues #97

Closed
aboba opened this issue Jun 15, 2014 · 5 comments
Closed

More WebIDL Issues #97

aboba opened this issue Jun 15, 2014 · 5 comments
Labels

Comments

@aboba
Copy link
Contributor

aboba commented Jun 15, 2014

From: Jason Ausborn [email protected]
Date: Sun, 15 Jun 2014 11:12:51 -0500
To: "[email protected]" [email protected]
URL: http://lists.w3.org/Archives/Public/public-ortc/2014Jun/0038.html

Possible Issue Location : 2.3 Interface Definition (interface
RTCDtlsTransport)

Possible Issue : Inside the interface we have a sequence, but
ArrayBuffer is not a WebIDL type, and we don't have it defined anywhere
else.


Possible Issue Location :
4.3 Infterface Definition (interface RTCIceTransportController)

Possible Issue : int is not a valid WebIDL type for addTransport (..., int
index) parameter.


Possible Issue Location 1 :
9.4 dictionary RTCRtcCodecCapability

Possible Issue Location 2:
9.7 dictionary RTCRtcCodecParameters

Possible Issue : Inside the dictionary we have a Dictionary type for
parameters, I don't think it is valid syntax to have a dictionary
type inside a dictionary.


Possible Issue Location : 11.3 Interface Definition (interface
RTCDataChannel)

Possible Issue 1: We have a Promise type used in the interface, but Promise
is not a WebIDL type, and we don't have a Promise type defined anywhere
else.

Possible Issue 2: We have an ArrayBuffer type used for RTCDataChannel.data,
but ArrayBuffer is not a WebIDL type, and we don't have ArrayBuffer defined
anywhere else.


Possible Issue Location : 13 Statistics API & 13.5 RTCStats Dictionary

Possible Issue : In section 13 we have a RTCStats interface, but we also
have a dictionary in 13.5 named RTCStats. Since they have the same name,
there is a clash. We might want to change the name of one of them.


Possible Issue Location 1:
6.3 Interface Definition (interface RTCRtpSender)

Possible Issue Location 2:
7.3 Interface Definition (interface RTCRtpReceiver)

Possible Issue : We don't have the MediaStreamTrack type defined anywhere.


HTH

@aboba
Copy link
Contributor Author

aboba commented Jun 20, 2014

  1. How about this instead of ArrayBuffer?

typedef octect[] ArrayBuffer;

  1. unsigned long instead of int
  2. A dictionary member can be a dictionary, as noted in WebIDL Section 3.3:

Each dictionary member (matching DictionaryMember) is specified as a type (matching Type) followed by an identifier (given by an identifier token following the type). The identifier is the key name of the key–value pair. If the Type is an identifier followed by ?, then the identifier MUST identify an interface, enumeration, callback function or typedef. If the dictionary member type is an identifier not followed by ?, then the identifier MUST identify any one of those definitions or a dictionary.

  1. Promises are a legal type, as noted here:
    http://heycam.github.io/webidl/#idl-promise
  2. We can rename RTCStats. One way:
interface RTCStatsBase {
    Promise<RTCStatsReport> getStats();
};

And then each interface that does stats derives from:

interface RTCRtpSender : RTCStatsBase {...}
  1. We should link to the MedaiStreamTrack definition.

@Jayzon
Copy link

Jayzon commented Jun 22, 2014

On the dictionary member, I believe that WebIDL may be expecting a dictionary type to be defined before it can be used as a member in another dictionary.

Currently :

dictionary RTCRtpCodecParameters {
   ...
    Dictionary parameters;
};

We might need to define the parameters dictionary type, and then place it as a dictionary member like this below.

dictionary RTCRtpCodecParameters {
   ...
    codecSpecificParameters parameters;
};

dictionary codecSpecificParameters {
   ...
};

However, the above may not be the case if the WebIDL syntax checker (that I am using) is not covering the current WebIDL syntax. The above change passes with the WebIDL online checker, and the previous syntax does not.

This is the WebIDL checker that I have been checking the syntax with.
http://www.w3.org/2009/07/webidl-check

Also, I was not able to get the syntax for a Promise type to work with the WebIDL checker yet, but I agree it is a legal type based on the specification, but I'm not confident with my understanding of the expected syntax currently.

@robin-raymond
Copy link
Contributor

@Jayzon do you know how anonymous dictionaries should be defined? I'm not sure myself with webidl.

@Jayzon
Copy link

Jayzon commented Jun 30, 2014

@robin-raymond Not fully sure based on the webidl spec. And to be honest, the way it is now may be correct. It is the webidl checker that pointed this out as an issue which doesn't seem to be fully up to date on the webidl spec.

I will try to find out more about this soon to know, but I wonder if we can just use an object type.

Example :

dictionary example {
object anonDictionary;
};

The above syntax does pass with the webidl checker.

@robin-raymond
Copy link
Contributor

recommend typedef object Dictionary; so meaning is understood.

@aboba aboba closed this as completed Jul 8, 2014
robin-raymond pushed a commit to robin-raymond/ortc that referenced this issue Jul 16, 2014
…3c#66

Added Identity support, as described in Issue w3c#78
Reworked getStats method, as described in Issue w3c#85
Removed ICE restart method described in Issue w3c#93
Addressed CNAME and synchronization context issues described in Issue w3c#94
Fixed WebIDL issues noted in Issue w3c#97
Addressed NITs described in Issue w3c#99
DTLS transport issues fixed as described in Issue w3c#100
ICE transport issues fixed as described in Issue w3c#101
ICE transport controller fixes made as described in Issue w3c#102
Sender and Receiver object fixes made as described in Issue w3c#103
Fixed RTCRtpEncodingParameter default issues described in Issue w3c#104
Fixed 'Big Picture' issues descibed in Issue w3c#105
Fixed RTCRtpParameter default issues described in Issue w3c#106
Added a multi-stream capability, as noted in Issue w3c#108
Removed quality scalability capabilities and parameters, as described in Issue w3c#109
Added scalability examples as requested in Issue w3c#110
Addressed WebRTC 1.0 Data Channel compatibility issue described in Issue w3c#111
Removed header extensions from RTCRtpCodecParameters as described in Issue w3c#113
Addressed RTP/RTCP non-mux issues with IdP as described in Issue w3c#114
Added getParameter methods to RTCRtpSender and RTCRtpReceiver objects, as described in Issue w3c#116
Added layering diagrams as requested in Issue w3c#117
Added a typedef for payload type, as described in Issue w3c#118
Moved onerror from the RTCIceTransport object to the RTCIceListener object as described in Issue w3c#121
Added explanation of Voice Activity Detection (VAD), responding to Issue w3c#129
Clarified the meaning of maxTemporalLayers and maxSpatialLayers, as noted in Issue w3c#130
Added RFC 6051 to the list of header extensions and removed RFC 5450, as noted in Issue w3c#131
Addressed ICE terminology issues, as described in Issue w3c#132
Separated references into Normative and Informative, as noted in Issue w3c#133
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants