amazon-chime-sdk-js
amazon-chime-sdk-js copied to clipboard
Conditional removal of H264 support is broken because H264 Camera sections are not found
What happened and what did you expect to happen?
The function DefaultSDP.findActiveCameraSection incorrectly looks for sendrecv however the presence of that field is not guaranteed. In fact, it is assumed to be the default value if recvonly is not present.
https://github.com/aws/amazon-chime-sdk-js/blob/128fa920d85c3817d42406b900b37c71e57a98bf/src/sdp/DefaultSDP.ts#L78
See https://datatracker.ietf.org/doc/html/rfc4566
If none of the attributes "sendonly", "recvonly", "inactive",
and "sendrecv" is present, "sendrecv" SHOULD be assumed as the
default for sessions that are not of the conference type
"broadcast" or "H332" (see below).
How to Fix the Issue
if (sec.indexOf('recvonly') === -1) {
hasCamera = true;
break;
}
Have you reviewed our existing documentation?
Reproduction steps
Use the project in Safari
Amazon Chime SDK for JavaScript version
2.24.0
What browsers are you seeing the problem on?
Safari
Browser version
15.1
Meeting and Attendee ID Information.
No response
Browser console logs
NA
@jonathan-emed Thanks for reporting. Could you add some details on how this affect your application use case?
https://github.com/aws/amazon-chime-sdk-js/blob/128fa920d85c3817d42406b900b37c71e57a98bf/src/task/SetLocalDescriptionTask.ts#L36
Tries to remove H264 from the SDP under certain conditions and it doesn't work on some browsers which omit the sendrecv attribute because it isn't necessary or required as per the RFC... which is Safari because Safari does not produce sendrecv and the browser that workaround is trying to work for is... Safari.
https://github.com/aws/amazon-chime-sdk-js/blob/128fa920d85c3817d42406b900b37c71e57a98bf/src/sdp/DefaultSDP.ts#L544
Therefore the entire workaround for the Safari problem with H264 doesn't actually work on versions of Safari which does not produce the sendrecv tag.
Hi @jonathan-emed, sorry for the delay, but appreciate seeing someone who enjoys looking at RFCs as much as I do :).
I'm onboard with your recommended change (i assume we would also want to check 'inactive' just in case the browser wants to throw that at the beginning of the function), but I am having difficulty reproducing. When I am using 15.2 MacOS safari (not sure there would be a difference between versions, I may be wrong though) I always see a=sendrecv. Additionally on the Munge SDP WebRTC sample it contains sendrecv (maybe its to do with https://github.com/aws/amazon-chime-sdk-js/blob/main/src/transceivercontroller/DefaultTransceiverController.ts#L340 )?
Anyways I don't mind making that modification, but am just curious about how to reproduce the issue. We had a good amount of testing to make sure that change fixed the iOS safari 15.1 bug and it didn't have issues.
@hensmi-amazon I reproduced the issue on an earlier version of Safari in December. It could have been the iPad/iPhone or Desktop. Honestly, I don't remember. I only discovered it because it wasn't actually removing the H264 like expected. So, a version of Safari did exist in December, which required this change to our fork.
Resolving as fixed