Re: [Gen-art] Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
On 2013-06-07 17:40, Elwyn Davies wrote: On Fri, 2013-06-07 at 16:05 +0200, Magnus Westerlund wrote: Appendix F: I missed that the text/parameter format appeared in the examples for GET_PARAMETER and SET_PARAMETER. It isn't stated in the definitions of these methods what encodings are acceptable for the message bodies that may go with these methods and their responses. Exactly, that is intentional. These methods can use anything that has a MIME type. Also it has HTTP's mechanisms for determining what formats one supports. Clearly the new text/parameter MIME format is one. Is it the only one? It is the only one defined by IETF for this purpose. That is why it is in the appendix so that RTSP users shall have something to define the parameters they want in. However, they have to accept the limitations of a basic text format. I guess you could use a application/json or an XML format but I guess these would also either have to be called out explicitly in the method descriptions or put in as feature extensions. This needs to be clarified in the method descriptions (s13). The upshot of all this is that I think Appendix F needs to be pulled back into Section 20 as currently it is the only way to encode the message bodies. I can agree that the format negotiation for the bodies of GET/SET_PARAMETER is not clear. I will look at proposing some improvements of the text related to the handling of method bodies and their format negotiation. Good. I don't see where the server tells what formats it accepts for either GET_PARAMETER or SET_PARAMETER. Also the Accept spec doesn't say anything about SET_PARAMETER. AFAICS the client could tell the server what formats it accepts as a side-effect of DESCRIBE but that's a bit arcane - and it is not clear how you would separate the formats for DESCRIBE from those for GET_PARAM and SET_PARAM. Yes, I think this negotiation should happen on per methods basis. However, I am not pulling in Appendix F. It is an optional to use format for parameter value pairs that can be used in these methods, it is not required. Nor, does the specification define any parameters that are transferred using this interface. The things that are in the appendix are not core protocol, they represent either informational things, like the examples and the state machine. The other appendices are normative definitions of particular choices for things that can be combined or used with RTSP, like RTP as media transport. OK, it is possible to use GET_PARAM for basic requirements without using message bodies, so you can get away without defining a lowest common denominator format. However, the use of message bodies for SET_PARAM is RECOMMENDED so it seems a little odd not to ensure that server and client will have at least one format in common. (And its not as if we are dealing with a hugely complicated bit of spec for text/parameters! :-) ). Then, given the example in GET_PARAMETER it seems sensible to advise implementing text/parameters as the default for GET_PARAM also. Sure, I personally don't have any issue with making it at least SHOULD implement text/parameters. But from my horizon a forward pointer with the normative requirements is sufficient to accomplish that. Cheers Magnus Westerlund -- Multimedia Technologies, Ericsson Research EAB/TVM -- Ericsson AB| Phone +46 10 7148287 Färögatan 6| Mobile +46 73 0949079 SE-164 80 Stockholm, Sweden| mailto: magnus.westerl...@ericsson.com --
Re: Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
Hi Elwyn, Many thanks for the detailed review. We will address the nits you have raised, but I cut them out of this reply to focus on the more substantial issues you have brought up. See inline below. On 2013-06-06 02:11, Elwyn Davies wrote: I am an additional Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq. Please resolve these comments along with any other Last Call comments you may receive. Document: draft-ietf-mmusic-rfc2326bis Reviewer: Elwyn Davies Review Date: 5 June 2013 IETF LC End Date: 5 JUne 2013 IESG Telechat date: (if known) - Summary: Almost ready. Generally this is an excellent and well written document, particularly given its size. There are a few minor issues to sort out mainly at the nit level and some consistency issues to fix up. The two issues that I have brought out below are really at what the IESG would call 'DISCUSS-DISCUSS' level. The issue of URI scheme redefinition may well have already been cleared by the URI expert - the draft does not do itself any favours here by failing to explain what the syntax changes are in s4.2; this raises immediate red flags that only start to fade a couple of hundred pages later. The rudimentary nature of the pipeline mechanism is carried over from RTSP 1.0. I'd like to be sure that this has been properly discussed in the WG as it looks pretty flaky to an outsider. There are several inconsistencies between various lists of headers that need sorting out and there is no definition of Proxy-Authorization in the ABNF. There are also a fairly large number of editorial nits but these are all localized and trivial to fix. Finally I have't had time to review the appendices (maybe there will be a follow up). Robert Sparks is the main gen-art reviewer for the document; he asked for additional eyes on the document given the size and his RAI background. Having (just) seen his review, I think we have both picked up on the URI scheme and pipeline issues. I am not sure that I concur with his view that there is significant normative material in the appendices - AFAICS the main protocol definition *is* in the main body of the document and the bits especiially in Appendices B and C are not the core of the protocol. However this is based on a very cursory glance through something like 75 pages of the document. However, I do concur with Robert's view that it needs a security expert to check the security stories. Major(ish) issues: s4.2: This section (re-)defines the URI schemes rtsp and rtsps. The last sentence of para 1 states that the 'details of the syntax' of the URIs 'have been changed'. Is this a reasonable thing to do? Has this been cleared with the URI expert reviewer? I am not entirely clear what the change involves and the draft doesn't explain exactly how the syntax has been altered and its consequences (if any) in s4.2. Some details are finally given in s22.14. These syntax changes where discussed in the reviews I got on the URI list. That resulted in the explicitness in the template, but I forgot about adding anything into Section 4.2. I see no issue with adding the following clarification to Section 4.2: The details of the syntax of rtsp and rtsps URIs has been changed from RTSP 1.0. These changes are: o Support for IPV6 literal in host part and future IP literals through RFC 3986 defined mechanism. o A new relative format to use in the RTSP protocol elements that is not required to start with /. Neither should have any significant impact on interoperability. If one is required to use IPv6 literals to reach an RTSP server, then that RTSP server must be IPv6 capable, and RTSP 1.0 is not a fully IPv6 capable protocol. Thus, RTSP 2.0 support will be needed anyway. The second change will only occur in RTSP messages, that are explicitly identified as being RTSP 2.0 messages, thus a RTSP 1.0 only agent will not be required to parse this variant. I hope this makes it clear that these syntax changes is unlikely to hurt the interoperability. Minor issues: s11, para 3: I guess that the WG decided that sticking with the RTSP 1.0 model of sending requests and worrying about success of prior prior pipelined requests was the right answer. I would have thought that it might have been helpful to provide qualifiers on the Pipelined-Requests header that allowed subsequent requests to be suppressed unless the previous command resulted in one of a given set of status codes with a 'reset' option to 'restart' the pipeline. It strikes me that this would avoid some irritating need to work out how to recover from arbitrary failures in a command chain in a client. I assume you mean this Section 12 paragraph: If a pipelined request builds on the successful completion of one or more prior requests the requester must verify that
Re: [Gen-art] Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
Hi Elwyn, On 2013-06-07 14:26, Elwyn Davies wrote: On Fri, 2013-06-07 at 11:35 +0200, Magnus Westerlund wrote: Hi Elwyn, Many thanks for the detailed review. We will address the nits you have raised, but I cut them out of this reply to focus on the more substantial issues you have brought up. .. and thanks for the quick response. See inline below. OK. I think the responses clear those issues. I have done a little bit more on the Appendices and liaised a bit with Robert Sparks. 'Highlights': Appendix F: I missed that the text/parameter format appeared in the examples for GET_PARAMETER and SET_PARAMETER. It isn't stated in the definitions of these methods what encodings are acceptable for the message bodies that may go with these methods and their responses. Exactly, that is intentional. These methods can use anything that has a MIME type. Also it has HTTP's mechanisms for determining what formats one supports. Clearly the new text/parameter MIME format is one. Is it the only one? It is the only one defined by IETF for this purpose. That is why it is in the appendix so that RTSP users shall have something to define the parameters they want in. However, they have to accept the limitations of a basic text format. I guess you could use a application/json or an XML format but I guess these would also either have to be called out explicitly in the method descriptions or put in as feature extensions. This needs to be clarified in the method descriptions (s13). The upshot of all this is that I think Appendix F needs to be pulled back into Section 20 as currently it is the only way to encode the message bodies. I can agree that the format negotiation for the bodies of GET/SET_PARAMETER is not clear. I will look at proposing some improvements of the text related to the handling of method bodies and their format negotiation. However, I am not pulling in Appendix F. It is an optional to use format for parameter value pairs that can be used in these methods, it is not required. Nor, does the specification define any parameters that are transferred using this interface. The things that are in the appendix are not core protocol, they represent either informational things, like the examples and the state machine. The other appendices are normative definitions of particular choices for things that can be combined or used with RTSP, like RTP as media transport. Appendix B: I appreciate that the state machine is illustrative and to an extent 'abstract'. However, the tables are really written to describe the state machine in the server: the action column mostly describes the events that come into the server (apart from the notify actions) - sending these 'events' are actions in the client. The 'real' state machine in the both the server and the client has a somewhat more complex form: I'd think that there was a STOPPED state in the server when the media has reached an end point and not been explictly paused and STARTING/PAUSING states in the client while the client waits for response to PLAY/PAUSE respectively. I think a little bit more explanation about the dual nature of the columns would solve the problem. There shouldn't be an issue to clarify this nature. Cheers Magnus Westerlund -- Multimedia Technologies, Ericsson Research EAB/TVM -- Ericsson AB| Phone +46 10 7148287 Färögatan 6| Mobile +46 73 0949079 SE-164 80 Stockholm, Sweden| mailto: magnus.westerl...@ericsson.com --
Re: [Gen-art] Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
On Fri, 2013-06-07 at 11:35 +0200, Magnus Westerlund wrote: Hi Elwyn, Many thanks for the detailed review. We will address the nits you have raised, but I cut them out of this reply to focus on the more substantial issues you have brought up. .. and thanks for the quick response. See inline below. OK. I think the responses clear those issues. I have done a little bit more on the Appendices and liaised a bit with Robert Sparks. 'Highlights': Appendix F: I missed that the text/parameter format appeared in the examples for GET_PARAMETER and SET_PARAMETER. It isn't stated in the definitions of these methods what encodings are acceptable for the message bodies that may go with these methods and their responses. Clearly the new text/parameter MIME format is one. Is it the only one? I guess you could use a application/json or an XML format but I guess these would also either have to be called out explicitly in the method descriptions or put in as feature extensions. This needs to be clarified in the method descriptions (s13). The upshot of all this is that I think Appendix F needs to be pulled back into Section 20 as currently it is the only way to encode the message bodies. Appendix B: I appreciate that the state machine is illustrative and to an extent 'abstract'. However, the tables are really written to describe the state machine in the server: the action column mostly describes the events that come into the server (apart from the notify actions) - sending these 'events' are actions in the client. The 'real' state machine in the both the server and the client has a somewhat more complex form: I'd think that there was a STOPPED state in the server when the media has reached an end point and not been explictly paused and STARTING/PAUSING states in the client while the client waits for response to PLAY/PAUSE respectively. I think a little bit more explanation about the dual nature of the columns would solve the problem. Appendix C: Pending. Regards, Elwyn On 2013-06-06 02:11, Elwyn Davies wrote: I am an additional Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq. Please resolve these comments along with any other Last Call comments you may receive. Document: draft-ietf-mmusic-rfc2326bis Reviewer: Elwyn Davies Review Date: 5 June 2013 IETF LC End Date: 5 JUne 2013 IESG Telechat date: (if known) - Summary: Almost ready. Generally this is an excellent and well written document, particularly given its size. There are a few minor issues to sort out mainly at the nit level and some consistency issues to fix up. The two issues that I have brought out below are really at what the IESG would call 'DISCUSS-DISCUSS' level. The issue of URI scheme redefinition may well have already been cleared by the URI expert - the draft does not do itself any favours here by failing to explain what the syntax changes are in s4.2; this raises immediate red flags that only start to fade a couple of hundred pages later. The rudimentary nature of the pipeline mechanism is carried over from RTSP 1.0. I'd like to be sure that this has been properly discussed in the WG as it looks pretty flaky to an outsider. There are several inconsistencies between various lists of headers that need sorting out and there is no definition of Proxy-Authorization in the ABNF. There are also a fairly large number of editorial nits but these are all localized and trivial to fix. Finally I have't had time to review the appendices (maybe there will be a follow up). Robert Sparks is the main gen-art reviewer for the document; he asked for additional eyes on the document given the size and his RAI background. Having (just) seen his review, I think we have both picked up on the URI scheme and pipeline issues. I am not sure that I concur with his view that there is significant normative material in the appendices - AFAICS the main protocol definition *is* in the main body of the document and the bits especiially in Appendices B and C are not the core of the protocol. However this is based on a very cursory glance through something like 75 pages of the document. However, I do concur with Robert's view that it needs a security expert to check the security stories. Major(ish) issues: s4.2: This section (re-)defines the URI schemes rtsp and rtsps. The last sentence of para 1 states that the 'details of the syntax' of the URIs 'have been changed'. Is this a reasonable thing to do? Has this been cleared with the URI expert reviewer? I am not entirely clear what the change involves and the draft doesn't explain exactly how the syntax has been altered and its consequences (if any) in s4.2. Some details are finally given in s22.14. These syntax changes where discussed in the reviews I got on the URI list. That resulted in the
Re: [Gen-art] Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
On Fri, 2013-06-07 at 16:05 +0200, Magnus Westerlund wrote: Appendix F: I missed that the text/parameter format appeared in the examples for GET_PARAMETER and SET_PARAMETER. It isn't stated in the definitions of these methods what encodings are acceptable for the message bodies that may go with these methods and their responses. Exactly, that is intentional. These methods can use anything that has a MIME type. Also it has HTTP's mechanisms for determining what formats one supports. Clearly the new text/parameter MIME format is one. Is it the only one? It is the only one defined by IETF for this purpose. That is why it is in the appendix so that RTSP users shall have something to define the parameters they want in. However, they have to accept the limitations of a basic text format. I guess you could use a application/json or an XML format but I guess these would also either have to be called out explicitly in the method descriptions or put in as feature extensions. This needs to be clarified in the method descriptions (s13). The upshot of all this is that I think Appendix F needs to be pulled back into Section 20 as currently it is the only way to encode the message bodies. I can agree that the format negotiation for the bodies of GET/SET_PARAMETER is not clear. I will look at proposing some improvements of the text related to the handling of method bodies and their format negotiation. Good. I don't see where the server tells what formats it accepts for either GET_PARAMETER or SET_PARAMETER. Also the Accept spec doesn't say anything about SET_PARAMETER. AFAICS the client could tell the server what formats it accepts as a side-effect of DESCRIBE but that's a bit arcane - and it is not clear how you would separate the formats for DESCRIBE from those for GET_PARAM and SET_PARAM. However, I am not pulling in Appendix F. It is an optional to use format for parameter value pairs that can be used in these methods, it is not required. Nor, does the specification define any parameters that are transferred using this interface. The things that are in the appendix are not core protocol, they represent either informational things, like the examples and the state machine. The other appendices are normative definitions of particular choices for things that can be combined or used with RTSP, like RTP as media transport. OK, it is possible to use GET_PARAM for basic requirements without using message bodies, so you can get away without defining a lowest common denominator format. However, the use of message bodies for SET_PARAM is RECOMMENDED so it seems a little odd not to ensure that server and client will have at least one format in common. (And its not as if we are dealing with a hugely complicated bit of spec for text/parameters! :-) ). Then, given the example in GET_PARAMETER it seems sensible to advise implementing text/parameters as the default for GET_PARAM also. /Elwyn
Gen-art additional LC review of draft-ietf-mmusic-rfc2326bis-34
I am an additional Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq. Please resolve these comments along with any other Last Call comments you may receive. Document: draft-ietf-mmusic-rfc2326bis Reviewer: Elwyn Davies Review Date: 5 June 2013 IETF LC End Date: 5 JUne 2013 IESG Telechat date: (if known) - Summary: Almost ready. Generally this is an excellent and well written document, particularly given its size. There are a few minor issues to sort out mainly at the nit level and some consistency issues to fix up. The two issues that I have brought out below are really at what the IESG would call 'DISCUSS-DISCUSS' level. The issue of URI scheme redefinition may well have already been cleared by the URI expert - the draft does not do itself any favours here by failing to explain what the syntax changes are in s4.2; this raises immediate red flags that only start to fade a couple of hundred pages later. The rudimentary nature of the pipeline mechanism is carried over from RTSP 1.0. I'd like to be sure that this has been properly discussed in the WG as it looks pretty flaky to an outsider. There are several inconsistencies between various lists of headers that need sorting out and there is no definition of Proxy-Authorization in the ABNF. There are also a fairly large number of editorial nits but these are all localized and trivial to fix. Finally I have't had time to review the appendices (maybe there will be a follow up). Robert Sparks is the main gen-art reviewer for the document; he asked for additional eyes on the document given the size and his RAI background. Having (just) seen his review, I think we have both picked up on the URI scheme and pipeline issues. I am not sure that I concur with his view that there is significant normative material in the appendices - AFAICS the main protocol definition *is* in the main body of the document and the bits especiially in Appendices B and C are not the core of the protocol. However this is based on a very cursory glance through something like 75 pages of the document. However, I do concur with Robert's view that it needs a security expert to check the security stories. Major(ish) issues: s4.2: This section (re-)defines the URI schemes rtsp and rtsps. The last sentence of para 1 states that the 'details of the syntax' of the URIs 'have been changed'. Is this a reasonable thing to do? Has this been cleared with the URI expert reviewer? I am not entirely clear what the change involves and the draft doesn't explain exactly how the syntax has been altered and its consequences (if any) in s4.2. Some details are finally given in s22.14. Minor issues: s11, para 3: I guess that the WG decided that sticking with the RTSP 1.0 model of sending requests and worrying about success of prior prior pipelined requests was the right answer. I would have thought that it might have been helpful to provide qualifiers on the Pipelined-Requests header that allowed subsequent requests to be suppressed unless the previous command resulted in one of a given set of status codes with a 'reset' option to 'restart' the pipeline. It strikes me that this would avoid some irritating need to work out how to recover from arbitrary failures in a command chain in a client. Nits/editorial comments: General: s/i.e./i.e.,/; s/e.g./e.g.,/ General: Consistent usage of octet vs. byte (octet preferred). General: Consistent use of '(session) keep alive' vs. '(session) keep-alive' General: Consistent use of Base64 vs BASE64. s1, para 2, last sentence: s/delivered as a time-synchronized streams/delivered as time-synchronized streams/ s1. last para: s/used terms/terms used/ s1, last para: Need to expand SDP acronym. s2, para 1: s/considered use cases/use cases considered/ s2.1, para 1: s/completely out of bands/completely out of band/ s2.1, next to last para: It would be useful to provide a pointer to where the two URI schemes are defined (s4.2?) so it is clear these are internally defined and one needn't go looking for another doc. s2.2, para 5: s/uses client-selected identifier/uses a client-selected identifier/ s2.2, para 5: For consistency there should be a reference for s18.32 with Pipelined-Requests. s2.2, last para: For consistency there should be a reference for s18.29 with Media-Range. s2.3, para 1: Expand SMPTE acronym. s2.3, para 3: Reference s13.5 for PLAY_NOTIFY s2.3, para 5: The server should also regularly send every 5 minutes the current media range in a PLAY_NOTIFY request. Shouldn't this be configurable? s2.3, last para: If the client waits too long before resuming the pause point, the content may no longer be available. In this case the pause point will be adjusted to the end of the available media. I know this is a subjective choice, but I would have thought adjusting to the beginning of the available media would be