drivers,server: Cap data read for a seg

[jailuthra]

What does this pull request do? Explain your changes. (required)

Cap the data read for a segment to prevent bad segments eating up a lot of memory.

Specific updates (required)

see the commit history

Currently the max segment size is set to ~300MB (at 8000kbps using the current max duration of 5minutes)

How did you test each of these updates (required)

• Manually tested on a local B/OT setup with a smaller limit set (2MB) and a bigger segment (2.4MB). The segment data read was being capped at the defined size.

With Limit Set:
B: I0404 22:11:10.044429 2542338 broadcast.go:372] Processing segment nonce=17696729379380681746 manifestID=test seqNo=1 dur=2 bytes=2097152
OT: [h264 @ 0x7f810403d640] error while decoding MB 60 59, bytestream -12
Transcoded Rendition Duration: 1.23 s

Without Limit Set:
B: I0404 22:12:42.698367 2543964 broadcast.go:372] Processing segment nonce=13032173926343571666 manifestID=test seqNo=1 dur=2 bytes=2405084
OT: No error
Transcoded Rendition Duration: 2.7 s

2. Added unit tests for Push API and B<->O serve segment in 9d23156

Does this pull request close any open issues?

Fixes #1523

Checklist:

• Read the contribution guide
• make runs successfully
• All tests in ./test.sh pass
• README and other documentation updated
• Pending changelog updated

[yondonfu]

After the update to start returning an error if the byte limit is exceeded while reading a request body, let's make sure to test for that error. I recommend small unit test cases as those should make it easy to construct request bodies that exceed the byte limit.

[iameli]

@yondonfu @jailuthra @darkdarkdragon I feel medium-strongly this should be a configurable parameter with a default value, perhaps -httpIngestMaxSize=20MiB or some such. Could use https://github.com/c2h5oh/datasize to make it easy to parse various sizes?

Just feels like the kind of thing I'd like the ability to do in a production setting to unbreak an integration that's ever-so-slightly over the limit.

[darkdarkdragon]

@iameli we also using this size on the O side. If we make this configurable then we should put this value in discovery protocol so O will clearly advertise its limits - or else B can get unexpected errors.

[jailuthra]

@iameli If we go with the bigger hardcoded limit of 5minutes as proposed by @yondonfu above, would we still need to increase it anytime soon-ish in a prod environment?

This can avoid changes on discovery side (for now), and I am definitely in for iterating over this in future!

[iameli]

Sure, that's reasonable.

[yondonfu]

Let's add some small tests for the new limiting behavior?

[yondonfu]

Latest changes look good to go for me pending the tests for the byte reading limiting behavior.

[jailuthra]

Latest changes look good to go for me pending the tests for the byte reading limiting behavior.

Added a couple of tests in 9d23156, rebased the PR and added a changelog entry.

Also created a fork debt issue for making the max segment size configurable.

[yondonfu]

LGTM

Fixed the comments, and got another approval too.