-
Notifications
You must be signed in to change notification settings - Fork 72
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
ffmpeg: bypass txcoder only for mpegts #206
Conversation
Does this mean that the transcoder won't be able handle successive MP4 segments if the first one is audio-only? |
Yep, deep dived into it further before commenting here, and turns out the MP4 format doesn't support dimensionless (0-frame) video streams at all. So there doesn't seem to be a way to copy an mpeg-ts segment with the 0-frame video stream, to an mp4 segment while still retaining the stream (it by default copies it to an audio-stream only segment, which doesn't work well with any player treating the segments as part of same stream) The current behaviour of the transcoder (as of this PR) with MP4 output is to skip the audio-only segments all together. It remains the same even if I allow MP4 segments to go through the The only way that we can support this in the long-term is by creating a dummy video stream and using that whenever the user supplied video stream is broken. That seemed simple to me initially but that will also have problems in case the user's source video is not the aspect ratio selected in the |
// we only bypass when a VideoProfile is set (i.e. not a copy/drop) and output format | ||
// is mpegts. | ||
// FIXME: Temporary fix. The transcoder is not the place for such hacky code | ||
p.Profile = VideoProfile{Name: p.Profile.Name, Format: FormatMPEGTS} |
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.
We'll skip the non-encoding related operations such as the scale & FPS filter if we initialize a new video profile here and only set the name/format. Is this desired behavior?
My initial thought is that we would want to still apply filters configured in the video profile fields, but just use the copy video & audio encoder. Or is it the case that the filters would not work properly for the first audio-only segment in a session so the only thing we can do is to pass the segment to the copy video & audio encoder?
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.
or is it the case that the filters would not work properly for the first audio-only segment in a session so the only thing we can do is to pass the segment to the copy video & audio encoder?
Yeah exactly! Filters get applied on decoded frames, and there are no video frames here for the audio-only segments :)
Although the subsequent normal segments will not go through this bypass function, and would get filtered/transcoded as usual.
There's no way to even set the filters on in the copy
mode https://stackoverflow.com/questions/53518589/how-to-use-filtering-and-stream-copy-together-with-ffmpeg
We can try enabling the audio-filters here (which don't really do much) but I wanna avoid that at first and only consider it if we face issues, because doing that will likely complicate the delicate re-muxing further.
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
Why?
Refer to livepeer/go-livepeer#1608 for details on why this was needed.
What?
Ensure the bypass implemented for audio-only segments at the start of a stream only happen for MPEG-TS muxer format and help out the transcoder by passing the out format through
VideoProfile.Format
. (go-livepeer uses.tempfile
extension because the segs might be .ts or .m4s)Note: This is a temporary hack, and I want to further emphasize that the transcoder should support a Go function (that calls Cgo ffmpeg api) to only check whether a bypass is needed. The actual bypass logic (and what options to tweak) should be maintained upstream either in the Broadcaster code (if needed for public network) or the API product code.