fix: Do not scale width by sarRatio, we already generate a pasp box with sarRatio information. #393
Conversation
lib/codecs/h264.js
Outdated
| }; | ||
|
|
||
| switch (data[0] & 0x1f) { | ||
| switch (event.nalUnitType) { |
There was a problem hiding this comment.
It's annoying to try and determine the type of nal unit if it's not one that we normally parse, so I added this.
There was a problem hiding this comment.
so now, for like sei_rbsp, the nalUnitType would end up being sei_rbsp but others would just pass along the hex value of it?
There was a problem hiding this comment.
Yeah, thinking about it again I think I will change it to nalUnitTypeCode that way type comparisons won't break down the line, since we expect it to not exist or be a string.
| @@ -470,7 +470,7 @@ H264Stream = function() { | |||
| profileIdc: profileIdc, | |||
| levelIdc: levelIdc, | |||
| profileCompatibility: profileCompatibility, | |||
| width: Math.ceil((((picWidthInMbsMinus1 + 1) * 16) - frameCropLeftOffset * 2 - frameCropRightOffset * 2) * sarScale), | |||
| width: (((picWidthInMbsMinus1 + 1) * 16) - frameCropLeftOffset * 2 - frameCropRightOffset * 2), | |||
There was a problem hiding this comment.
We shouldn't need to Math.ceil if we are not scaling now.
There was a problem hiding this comment.
I think the original reason that we did scaling is because we did not encode a pasp atom with sarRatio information when mux.js was created. So We had to encode width and height as their real values at all times. I think we should have been scaling height in some scenarios too. Overall we won't have to do any of that now as the decoder will see the pasp and scale accordingly. We just have to report the values that exist.
| @@ -365,7 +367,7 @@ H264Stream = function() { | |||
| picHeightInMapUnitsMinus1, | |||
| frameMbsOnlyFlag, | |||
| scalingListCount, | |||
| sarRatio, | |||
| sarRatio = [1, 1], | |||
There was a problem hiding this comment.
default to a 1 to 1 aspect ratio, ie width and height are exact values.
While this doesn't give us the absolute width when we have a
sarRatio, we don't want to use the absolute width. This is because we already encode thesarRatioin thepaspatom of our mp4 output. By encoding thesarRatioand an absolute width, the width is skewed even further unless the decoder looks directly at the nal units and calculates it itself. Example:sarRatiois0.5widthis640withoutsarRatioHow it is now with
sarRatiowidth would get set to320and then when the decoder see's thepaspatom withsarRatioit will think that width is160. Some decoders will ignore all these values on use nal units to calculate the width, but mobile Samsung Chrome on Android 11 will not. This causes a media decode error as the actual width of the video is320.With this change width will be set to
640and thesarRatiopaspencoding will make the decoder set the width to320