Update to latest LPMS with audio-only segment support

[jailuthra]

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

Updates the LPMS module to the latest commit. See livepeer/lpms#204 for details.

Specific updates (required)

• 5185ec0 - Update the lpms module to the latest commit
• 8e1abf4 - Use .tempfile.ts extension instead of simple .tempfile for the temporary in/out segs on the transcoder. The audio-only segments were failing to transcode without this as FFmpeg did not know what format to output to without the file extension help.

How did you test each of these updates (required)

On local B/O/T setup:

• Manually tested a normal RTMP stream is transcoding to multiple profiles
• Manually tested the original failing segment is transcoded correctly when sent via curl as the first segment of a stream.

Does this pull request close any open issues?

Closes #1607

Checklist:

• N/A README and other documentation updated
• Node runs in OSX and devenv linux
• All tests in ./test.sh pass

[yondonfu]

The audio-only segments were failing to transcode without this as FFmpeg did not know what format to output to without the file extension help.

Do you know why audio-only segments with the .tempfile extension fail, but regular segments with the .tempfile extension do not fail? Is it related to the use of the copy audio encoder for the first audio-only segment of a session?

[jailuthra]

Do you know why audio-only segments with the .tempfile extension fail, but regular segments with the .tempfile extension do not fail? Is it related to the use of the copy audio encoder for the first audio-only segment of a session?

@yondonfu Yes that is my suspicion as well but I'm not sure.

Looking through the LPMS go module again, I think we can set the muxer/format name through the TranscodeOptions.Muxer option instead, which will ultimately call the FFmpeg's format guesser av_guess_format with the muxer name we pass (mpegts) and random.tempfile which should also work out fine without adding the .ts file extension

But the extension change still seems like a cleaner way to do it as it's done for input segments as well which can't be configured with a transcoder option. Unless there was a specific reason we were keeping the segments extension-less?

[yondonfu]

Unless there was a specific reason we were keeping the segments extension-less?

Hm looks like the file type extension was removed in favor of .tempfile in adfa5be which was a part of the MP4 support PR. IIUC the reason to use .tempfile instead of a file type extension is because the temporary file is not necessarily a mpegts segment - it could also be a mp4 segment.

Would the switch to a .tempfile.ts extension in this PR break MP4 transcoding of audio-only segments? If so, then we probably want to use a different solution to handle audio-only segments with the .tempfile extension.

[jailuthra]

IIUC the reason to use .tempfile instead of a file type extension is because the temporary file is not necessarily a mpegts segment - it could also be a mp4 segment.

Ah that makes sense

Would the switch to a .tempfile.ts extension in this PR break MP4 transcoding of audio-only segments?

No idea, will need to deep dive and check. I do think we shouldn't mess with the input segment's extension now that I know it might mess our own detection done by this hashmap, I'll update the commit for that at least. But re: changing the extension for the output segment - it does not seem to break any of the existing unit tests, but I wonder if this is even validated. Let me try to manually transcode with MP4.

If so, then we probably want to use a different solution to handle audio-only segments with the .tempfile extension.

I agree. Phew just when I thought it would work with a simple bypass!

[jailuthra]

1. Reverted the .tempfile.ts changes
2. Opened a PR in LPMS to only do the bypassing for MPEG-TS (and pass the muxer opt to transcoder there itself) ffmpeg: bypass txcoder only for mpegts lpms#206
3. Updated the go module for LPMS to the new commit under that PR

Current Status: Streams don't fail for our usecase of initial-segments-with-0-video-frames. But playing such streams back is weird on different players, as many rely on the first segment's video stream to figure out the decode settings. @iameli I would highly recommend to check if this even helps the API product's usecase.

I now wonder if this approach (of using copy-transcoder for the 0-frame segments) even makes sense. Maybe the transcoder should just provide a profile option to force explicit black frames and let the upstream handle the logic of checking for bad video-stream and forcing that option for particular segments. But supporting blank-frames in the transcoder wouldn't be trivial either, will need to create a dummy source using libavfilter etc.

[jailuthra]

1. Reverted the bypass code altogether in LPMS see ffmpeg: Throw error instead of bypassing for audio-only stream lpms#207 (Still detecting audio-only segments and throwing a nice error)
2. Squashed all commits and updated LPMS module to livepeer/lpms@d5c85d8
3. Tested the go-livepeer node off-chain to see that the B errors out quickly with the new error when we send an audio-only segment at the start of a stream

I0924 19:16:17.685037  259653 segment_rpc.go:382] Submitting segment nonce=14989350065758642986 manifestID=test seqNo=1 bytes=50948 orch=https://0.0.0.0:8935
I0924 19:16:17.688265  259653 segment_rpc.go:416] Uploaded segment nonce=14989350065758642986 manifestID=test seqNo=1 orch=https://0.0.0.0:8935 dur=3.192899ms
E0924 19:16:17.691270  259653 segment_rpc.go:448] Transcode failed for segment nonce=14989350065758642986 manifestID=test seqNo=1 orch=https://0.0.0.0:8935 err=No video parameters found while initializing stream

[yondonfu]

LGTM 🚢