Add support for a=imageattr (RFC 6236) #61
Conversation
- add export.parseImageattrParams()
lib/parser.js
Outdated
| list.push(params); | ||
| }); | ||
|
|
||
| return list; |
There was a problem hiding this comment.
the forEach in here should be a map so you don't have to think about the list temporary.
| t.deepEqual(imageattr1RecvParams, [ | ||
| {'x': 1280, 'y': 720}, | ||
| {'x': 320, 'y': 180}, | ||
| ], 'video 1st imageattr recv params'); |
lib/grammar.js
Outdated
| //a=imageattr:* send [x=800,y=640] recv * | ||
| //a=imageattr:100 recv [x=320,y=240] | ||
| push: 'imageattrs', | ||
| reg: /^imageattr:(\d+|\*)(?:[\s\t]+send[\s\t]+(\*|\[\S+\](?:[\s\t]+\[\S+\])?)+)?(?:[\s\t]+recv[\s\t]+(\*|\[\S+\](?:[\s\t]+\[\S+\])?)+)?/, |
There was a problem hiding this comment.
that's one solid regex - i'd consider breaking this into multiple lines with comments
There was a problem hiding this comment.
A literal regular expression cannot be broken into multiple lines. The only workaround is using a RegExp object, but not sure if that would work.
There was a problem hiding this comment.
yeah, you'd have to use that instead of literal, but they are both regexes, it should work fine.
that said, it's not that bad, it just looks big at first sight due to all the repeated [\s\t] whitespace matchers. feel free to merge this now that you added the other changes.
|
nice addition, thanks a lot. left some suggestions, nothing super important though. |
| '(?:[\\s\\t]+send[\\s\\t]+(\\*|\\[\\S+\\](?:[\\s\\t]+\\[\\S+\\])?)+)?' + | ||
| //recv [x=330,y=250] | ||
| '(?:[\\s\\t]+recv[\\s\\t]+(\\*|\\[\\S+\\](?:[\\s\\t]+\\[\\S+\\])?)+)?' | ||
| ), |
imageattrsfora=imageattrlines (RFC 6236)export.parseImageattrParams()