fix: prevent skipping frames on garbage adts data #390
Conversation
| } | ||
|
|
||
| // remove processed bytes from the buffer. | ||
| buffer = buffer.subarray(i); |
There was a problem hiding this comment.
remove the bytes that we processed from the buffer.
| @@ -64,7 +64,7 @@ AdtsStream = function(handlePartialSegments) { | |||
|
|
|||
| // Prepend any data in the buffer to the input data so that we can parse | |||
| // aac frames the cross a PES packet boundary | |||
| if (buffer) { | |||
| if (buffer && buffer.length) { | |||
There was a problem hiding this comment.
Buffer is no longer set to undefined if we don't have bytes left, I was torn on initializing it to an empty uint8 array and always concating with buffer to make all of this code simpler.
lib/codecs/adts.js
Outdated
| // we use i + 3 here because we want to be able to parse the frame length | ||
| // from the header. If we don't have enough bytes to do that, than we | ||
| // definitely won't have a full frame. | ||
| while ((i + 3) < buffer.length) { |
There was a problem hiding this comment.
Since we use i + 5 in determining frameLength below (and i + 6 below that), wouldn't we require at least that much?
There was a problem hiding this comment.
you're right, I only looked at the first byte that it asks for, I will update this.
brandonocasey
force-pushed
the
fix/adts-parsing-loop
branch
from
e5c0879
to
a469880
Compare
| return; | ||
| // If we don't have enough data to actually finish this ADTS frame, | ||
| // then we have to wait for more data | ||
| if ((buffer.byteLength - i) < frameLength) { |
There was a problem hiding this comment.
We should probably move this check up to just after determining the frame length (before the sample count) to prevent errors addressing i + 6
There was a problem hiding this comment.
changed the while loop condition to i + 7 so that we won't even run the loop if we don't have enough bytes. That way and we can keep all the parsing logic together.
Co-authored-by: Garrett Singer <gesinger@gmail.com>
Right now we run into an issue if adts contains data between sync words that is not part of a frame.
Example
We have an adts file where bytes 0-5 are garbage, and after that the file is like normal. This is what would happen right now:
iis now incremented up to5.ito0buffer[i]for a syncword butiis still5so we skip everything else in the buffer until the next syncword and further aggravate the problem by incrementingisome more.