-
Notifications
You must be signed in to change notification settings - Fork 175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MP4 support for HTTP push. #1429
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mp4
muxer/demuxer should be added to install_ffmpeg.sh
@darkdarkdragon Ah yes. Added a unit test that will iterate through all our formats, to catch this type of oversight in the future: 8abe8ab . That failed exactly as expected. Added mp4 to the muxer/demuxer and tests pass now. 06b8783 |
Rebased to fix up a merge conflict. Git SHAs have changed but commit structure is otherwise the same in case someone was in the middle of reviewing things (eg, still contains the pending fixups from #1429 (comment)) |
2c075d9 Updated to incorporate latest LPMS, including the libavfilter performance fix and MP4 support among other things: livepeer/lpms#176 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Left one note about a comment about a possible bug in existing code. I'll leave it up to you whether to address it in this PR or separately (I noted that I'm fine with doing it separately).
Most of the changes are in the following areas: * Adds a format -> extension lookup function * Adds a extension -> format inverse lookup function * Adds a format -> mime type lookup function * Protocol Buffers changes, and conversions to/from VideoProfile * Slotting in the lookups and error handling where appropriate * Lots and lots of tests. The extensive testing and strict error handling is both to codify intended behavior and to ensure misuse resistance to further changes.
The extension doesn't matter, and may no longer be mpegts.
This allows us to fail out quickly on orchestrators that do not recognize the FullProfile field.
Only use this field for non-mpegts profiles in order to maintain compatibility with older orchestrators. If an older orchestrator receives a request for non-mpegts transcoding, it will fail out.
What does this pull request do? Explain your changes. (required)
This implements most of #1418 . The exception is that it does not implement output configuration via webhook. The output can be set to MP4 via the HTTP push file extension. This can be quickly tested on the broadcaster via:
and the results inspected via
curl
the resulting playlist(s) at http:localhost:8935/live/mp4.m3u8 . ffplay of individual segments will work as well.Specific updates (required)
server: Take MP4s as input for HTTP push …
Most of the changes are in the following areas:
The extensive testing and strict error handling is both to codify
intended behavior and to ensure misuse resistance to further changes. 949a063
The file extension of temp files mostly doesn't matter for us anymore, so renamed the temp files we generate in order to avoid future confusion about the file type. MP4 support for HTTP push. #1429
Added documentation 3959817
Updated LPMS to Add MP4 format to VideoProfile. lpms#176 (needs to be merged prior to this ; any changes there will be updated here in go.mod)
How did you test each of these updates (required)
Does this pull request close any open issues?
Fixes #1418
Checklist:
./test.sh
pass