mbebenita / Broadway

A JavaScript H.264 decoder.
Other
2.73k stars 424 forks source link

Make MAX_STREAM_BUFFER_LENGTH configurable #206

Open Efulefu opened 5 years ago

Efulefu commented 5 years ago

Using Broadway JS over a LAN, I've been able to choose higher resolutions and quality/bitrate settings than would be recommended for a web application.

As such I've ran into issues overflowing the Decoder.streamBuffer with huge frames.

Directly increasing MAX_STREAM_BUFFER_LENGTH in Decoder.js seems to correctly fix the overflow issue, as such I was wondering if it would be possible to make it configurable rather than a hard-coded constant.

The change seems pretty trivial from my understanding of the source code, could changing MAX_STREAM_BUFFER_LENGTH have any weird side-effects elsewhere in the library? I'm also asking this because of the other issue I've raised ( #205 ), since it's hard to tell as a new user of the library whether those could be related or not!

Just as with the other issue, I'd be willing to prepare and submit a PR, especially if you can confirm the triviality of the change!

soliton4 commented 5 years ago

i like your enthusiasm but i have to correct some missunderstandings

the MAX_STREAM_BUFFER_LENGTH configures a buffer length for 1 nal unit. there should never be a nal unit longer than 1mb since i believe thats what the standard defines (could be wrong) so if you have nal units longer than 1mb that would be an interesting case

i suspect however that you somehow combine more than 1 nal into one large binary that exceeds 1mb. that is wrong for several reasons and you should always split your nals before passing them on to broadway

the decdoing of the nals itself is as far as i know infinetly buffered so no need to increase that

Efulefu commented 5 years ago

Oh I see, indeed I misunderstood the role of the streamBuffer array, thank you.

Yes there's a very good chance I'm making a mistake in my NAL splitting code, I'll go through it! However I can't quite find a good reference for NALU maximum sizes, either ctrl-f'ing the ITU specs, or googling it - the most relevant information I have found is those SO discussions: NALU max size Limiting NALU size in ffmpeg

I guess it should help to reencode with MTU-sized NALUs either way, and that should also solve this issue, but I'll admit I was hoping to find documentation I could use to sanity check my NAL splitter's output as well.