Giter Site home page Giter Site logo

Comments (10)

garethsb avatar garethsb commented on September 1, 2024

Its description of the fmtp attribute is ambiguous:

a=fmtp:<format> <format specific parameters>

Is that whitespace between the ">" and the "<" required to be a single space?

Yes, the SDP spec is unclear on that like much else. When writing SDP files the best one can do is usually to follow the spec where it's clear and convention where it isn't. I'd not previously seen an SDP file with multiple spaces there.

Also, a comment regarding logging: It took me a while to track down this problem, simply because I was looking at the wrong SDP when trying to figure out what was wrong. It's not clear to me how the code might be changed to include the offending fmtp line in the exception, if that's even possible, but that would have reduced my debugging time a fair amount.

I think the main problem is that there wasn't an exception during parsing. The parser just happily produced a list of parameters the first of which was called " sampling" with a leading space. By the point in the code that caused trouble, the original SDP lines have long gone.

If we were to make the extra space cause parsing to fail, you'd have got a message that told you the offending line number. So that's one approach.

Aside from that, if it is [isn't?] compliant, should we care, or should the parser be a bit more forgiving of whitespace in the fmtp attribute?

That's the flip-side. In the case of a well-written spec, implementing a strict parser encourages everyone to comply. In this case however, where the spec is unclear and it's pretty easy to argue that the intent of the file is still clear with the extra space, then selfishly, a more lax parser would help, wouldn't it, despite that it promulgates the 'error'?

It's not just the beginning and end of the format specific parameters in the fmtp attribute where whitespace handling could be more forgiving. Should the parser allow a separator of a single semi-colon with no space, or a semi-colon and forty-two spaces? What about that quirky "required" space after a=source-filter:. And there are other places where the separator in a list is a single space in the spec, but intent would still be clear with multiple spaces.

I wonder how forgiving would be just the right amount of forgiving...

Thoughts?

from nmos-cpp.

billt-hlit avatar billt-hlit commented on September 1, 2024

β€œTo err is human; to forgive, divine.”

2110-20 does not define "whitespace" (surprise!), but in everything I wrote above I was assuming "whitespace" means "[ \t]+" (and maybe end-of-line markers, although that gets hairy) and not "a single whitespace character". So, this becomes not merely a question of forgiveness but a question of definitions. I'm guessing my interpretation is a minority opinion, but I'm sure it's the right one. 🀑

I don't have much experience with the source-filter attribute (haven't bothered to implement it). That said, given that the RFC has an ABNF grammar, I don't see there is room or need for wiggle.

[edit: Confusion about logging deleted]

from nmos-cpp.

garethsb avatar garethsb commented on September 1, 2024

2110-20 does not define "whitespace" (surprise!),

Yes, utterly unexpected that. ;-)

RFC4566 Section 5 does say:

An SDP session description consists of a number of lines of text of
the form:

   <type>=<value>

where <type> MUST be exactly one case-significant character and
<value> is structured text whose format depends on <type>. In
general, <value> is either a number of fields delimited by a single
space character
or a free format string, and is case-significant
unless a specific field defines otherwise. Whitespace MUST NOT be
used on either side of the "=" sign.

I read that to say that unless specified differently, exactly one SP must be used as a field separator. Most of the SDP lines have an ABNF grammar that backs that interpretation, but, as you said, fmtp doesn't.

RFC4566 doesn't define "whitespace" either; in fact the above paragraph is the only use of the term! For what it's worth, the core terminals of IETF ABNF are defined by RFC4234 Appendix B.1. It defines a WSP (whitespace) character, as you suggested, as either SP (space) or HTAB (horizontal tab).

So, I think the conclusion is that in most cases, expecting and accepting only a single space is correct, but that in the case of fmtp, a forgiving parser should allow 1*WSP (i.e. one or more) before, between, and after, the format-specific-parameters.

from nmos-cpp.

garethsb avatar garethsb commented on September 1, 2024

OK, that was a bit painful. @billt-hlit I'd be really grateful for some additional testing!

from nmos-cpp.

billt-hlit avatar billt-hlit commented on September 1, 2024

The video fmtp attribute parsing looks good. On the other hand, you need to do the same thing for "i=" that you did for "s=".

from nmos-cpp.

garethsb avatar garethsb commented on September 1, 2024

Is that true? The session name s= line is required and has the special language about using a single space as a minimum. The session/media information i= lines are optional, so no such special treatment seems warranted?

from nmos-cpp.

billt-hlit avatar billt-hlit commented on September 1, 2024

Oh, sorry, I didn't realize that "byte-string" required a least one character. Seems like an unnecessary restriction, but the grammar is clear about it. So no, don't worry about it.

from nmos-cpp.

garethsb avatar garethsb commented on September 1, 2024

Agreed.

I'm sure more edge cases are going to turn up. πŸ˜”

Closing... for now...

from nmos-cpp.

billt-hlit avatar billt-hlit commented on September 1, 2024

Another obscure edge case from RFC 4570:

The fourth sub-field, <src-list>, is the list of source
hosts/interfaces in the source-filter, and consists of one or more
unicast addresses or FQDNs, separated by space characters.

A naive reading (why is everyone looking at me? 😊) might interpret this as saying there are any number of space characters between each of the source addresses. On the other hand, the rest of the description in RFC 4750 falls under the "in general" section of RFC 4566 Gareth quoted above.

Opinions? I'm tempted to stick to the single-space interpretation here.

from nmos-cpp.

garethsb avatar garethsb commented on September 1, 2024

Hi Bill,

Luckily in this case the ABNF in Appendix A comes to the rescue:

src-list =       *(unicast-address SP) unicast-address

from nmos-cpp.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.