Skip to content

Add support for specifying output file format and audio sync option #2892

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 23, 2020
Merged

Add support for specifying output file format and audio sync option #2892

merged 5 commits into from
Nov 23, 2020

Conversation

beardypig
Copy link
Member

The change is introducing two new optional command line options:

  • --ffmpeg-fout: The output file format when muxing with ffmpeg
  • --ffmpeg-no-start-at-zero: When used with ffmpeg and copyts, disables automatically shifting input timestamps so they start at zero

Both options defaults are keeping the predefined values, hence backwards compatibility is enforced.

@lcapriotti @beardypig
Add support for specifying output file format and audio sync option
cb45427 
Co-Authored-By: beardypig <beardypig@protonmail.com>
@beardypig
Copy link
Member Author

I added some extensive changes that couldn't be done with suggestions. I added some tests and fixed some bugs that were exposed by the tests...

@beardypig beardypig requested a review from back-to April 13, 2020 16:15
@beardypig
Copy link
Member Author

Replaces #2774

@codecov
Copy link

codecov bot commented Apr 13, 2020 •
edited
Loading

Codecov Report

Merging #2892 (c61ed5b) into master (00b6929) will decrease coverage by 10.89%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master    #2892       +/-   ##
===========================================
- Coverage   63.91%   53.01%   -10.90%     
===========================================
  Files         444      252      -192     
  Lines       20292    15783     -4509     
  Branches     2774        0     -2774     
===========================================
- Hits        12969     8367     -4602     
- Misses       6993     7416      +423     
+ Partials      330        0      -330

back-to
back-to reviewed Apr 15, 2020
View reviewed changes
src/streamlink/stream/ffmpegmux.py Outdated
@@ -91,13 +91,14 @@ def __init__(self, session, *streams, **options):
for stream, np in
zip(self.streams, self.pipes)]

ofmt = options.pop("format", "matroska")
ofmt = options.pop("format", session.options.get("ffmpeg-fout"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the user command can't work because format is not None

$ streamlink https://www.zdf.de/live-tv --ffmpeg-fout "matroska"
[stream.ffmpegmux][debug]

ffmpeg command: /usr/bin/ffmpeg -nostats -y
-i /tmp/ffmpeg-8930-729 -i /tmp/ffmpeg-8930-644
-c:v copy -c:a copy -map 0:v? -map 0:a?
-map 1:a -f mpegts pipe:1

same for vcodec acodec start_at_zero

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you're right. I have it backwards :/

It should go ffmpeg-fout -> options.format -> default

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some more tests, and defaults in the class - it should behave as expected now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry but I see you are using "matroska" for mpegts
what would be the value to fill in for "flv" ?

Copy link
Member

@gravyboat gravyboat Sep 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issues is ancient, why are you bumping it? It is now locked.

@back-to
Copy link
Collaborator

back-to commented Apr 15, 2020

setting the value might work, but is there an actual working use case

not sure which formats are valid https://ffmpeg.org/ffmpeg-formats.html
but most of the time it will just fail.

@lcapriotti what was your intention with this feature?

[cli][error] Try 1/1: Could not open stream <Stream()> (No data returned from stream)
error: Could not open stream <Stream()>, tried 1 times, exiting

...

[stream.ffmpegmux][error] Pipe copy aborted: /tmp/ffmpeg-23548-515
[cli][debug] Writing stream to output
[stream.ffmpegmux][debug] Closing ffmpeg thread
[stream.ffmpegmux][debug] Closed all the substreams
[cli][info] Stream ended
[cli][info] Closing currently open stream...
[stream.ffmpegmux][debug] Closing ffmpeg thread
[stream.ffmpegmux][debug] Closed all the substreams
[stream.ffmpegmux][debug] Closing ffmpeg thread
[stream.ffmpegmux][debug] Closed all the substreams

@lcapriotti
Copy link
Contributor

@lcapriotti what was your intention with this feature?
I am using my forked version to stream from a Swiss site (Wilmaa), because master was not giving proper results for these MPEG-DASH streams (audio delay); I also am in need to have TS streams as output hence the ffmpeg options.

This was referenced Jul 6, 2020
Merged
Closed
@back-to back-to mentioned this pull request Nov 14, 2020
Closed
1 task
@back-to back-to added the PR: squash commits Commits need to be squashed as a single commit - eg. multiple commits for a single component label Nov 21, 2020
back-to
back-to approved these changes Nov 21, 2020
View reviewed changes
Copy link
Collaborator

@back-to back-to left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would merge it like this if there are no concerns

PR is required for #2747

@gravyboat
Copy link
Member

@back-to It needs a rebase. I closed the other PR for the time being.

@back-to back-to merged commit 568c89d into streamlink:master Nov 23, 2020
@gravyboat gravyboat mentioned this pull request Nov 23, 2020
Closed
Billy2011 pushed a commit to Billy2011/streamlink-27 that referenced this pull request Nov 24, 2020
@beardypig @lcapriotti @Billy2011
[PATCH] stream.ffmpegmux: Add support for specifying output file form…
672ce5f 
…at and audio sync option (streamlink#2892)

The change is introducing two new optional command line options:

    `--ffmpeg-fout`: The output file format when muxing with ffmpeg
    '--ffmpeg-no-start-at-zero': When used with ffmpeg and copyts, disables automatically shifting input timestamps so they start at zero

Both options defaults are keeping the predefined values, hence backwards compatibility is enforced.

Co-Authored-By: beardypig <beardypig@protonmail.com>
Co-authored-by: Luigi Capriotti <l.capriotti@tiscali.it>
Billy2011 pushed a commit to Billy2011/streamlink-27 that referenced this pull request Nov 28, 2020
@beardypig @lcapriotti @Billy2011
[PATCH] stream.ffmpegmux: Add support for specifying output file form…
58ba777 
…at and audio sync option (streamlink#2892)

The change is introducing two new optional command line options:

    `--ffmpeg-fout`: The output file format when muxing with ffmpeg
    '--ffmpeg-no-start-at-zero': When used with ffmpeg and copyts, disables automatically shifting input timestamps so they start at zero

Both options defaults are keeping the predefined values, hence backwards compatibility is enforced.

Co-Authored-By: beardypig <beardypig@protonmail.com>
Co-authored-by: Luigi Capriotti <l.capriotti@tiscali.it>
@bastimeyer bastimeyer mentioned this pull request Dec 15, 2020
Merged
@streamlink streamlink locked as resolved and limited conversation to collaborators Sep 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gravyboat gravyboat gravyboat left review comments

@campones campones campones left review comments

@back-to back-to back-to approved these changes

Assignees
No one assigned
Labels
enhancement PR: squash commits Commits need to be squashed as a single commit - eg. multiple commits for a single component
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@beardypig @back-to @lcapriotti @gravyboat @campones