Skip to content
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

use scaletempo2 by default #8376

Closed
532910 opened this issue Dec 8, 2020 · 10 comments
Closed

use scaletempo2 by default #8376

532910 opened this issue Dec 8, 2020 · 10 comments

Comments

@532910
Copy link

532910 commented Dec 8, 2020

Manpage says:

If --audio-pitch-correction (on by default) is used, playing with a speed higher than normal automatically inserts the scaletempo audio filter.

and if playback speed will be changed (with default [ ] { } controls for example) scaletempo will be used. But scaletempo2 sounds significant better. Is there something that prevents using scaletempo2 instead of scaletempo by default?

mpv 0.33.0 from Christian's repository (deb-multimedia), debian sid

@Hrxn
Copy link
Contributor

Hrxn commented Dec 8, 2020

I don't think so, but scaletempo2 (which is based on pitch correction taken from the Chromium repo, as far as I know) hasn't been merged that long ago, I think that's why.

@adisib
Copy link

adisib commented Dec 8, 2020

I've been running a patch to auto-insert scaletempo2 since it was added, and have experienced no issues at all with it. In case that adds any confidence towards making this change.

@v-fox
Copy link

v-fox commented Dec 11, 2020

I've been running a patch to auto-insert scaletempo2 since it was added, and have experienced no issues at all with it. In case that adds any confidence towards making this change.

Could you link or post that patch here, please ? Or just forcing autoload by default with af-add=scaletempo2 in config would prevent it being auto-replaced with scaletempo ?

jeeb added a commit to jeeb/mpv that referenced this issue Jan 20, 2021
Part 1 of "look how well it performs, then start cleaning up the
old one."

Ref mpv-player#8376
jeeb added a commit to jeeb/mpv that referenced this issue Jan 20, 2021
Part 1 of "look how well it performs, then start cleaning up the
old one."

Closes mpv-player#8376
@jeeb
Copy link
Member

jeeb commented Jan 20, 2021

Opened a PR to do this since the documentation notes scaletempo2 as being superior to scaletempo. Original scaletempo will at least for the short-term future stick around, but if we do not see any larger issues we'll start removing the original scaletempo filter.

Brief testing by a volunteer showed one issue being that apparently speeds below 0.25x silence the audio. Which does not block initial switch-over of the automatic filter logic.

@MerkaST
Copy link

MerkaST commented Jan 20, 2021

Muting below 0.25x and above 8x speed is default behaviour and can be changed via filter options. There does seem to be a bug related to exceeding 8x when the maximum is set higher (which also has an open pull request in which I see you just posted as well).

@jeeb jeeb closed this as completed in b0b37df Feb 14, 2021
zsoltiv pushed a commit to zsoltiv/mpv that referenced this issue Mar 2, 2021
Part 1 of "look how well it performs, then start cleaning up the
old one."

Closes mpv-player#8376
Dudemanguy pushed a commit to Dudemanguy/mpv that referenced this issue May 18, 2021
Part 1 of "look how well it performs, then start cleaning up the
old one."

Closes mpv-player#8376
@delthas
Copy link

delthas commented Oct 28, 2023

Muting below 0.25x and above 8x speed is default behaviour and can be changed via filter options.

FWIW this is:

mpv --af=scaletempo2=min-speed=0.0 file.mp4

@lcksk
Copy link
Contributor

lcksk commented Apr 19, 2024

I apologize if there might have been some noise. Compared to scaletempo, it appears that scaletempo2 lacks internal support for the af_format_s16 format, which is nearly ubiquitous in the Android platform for OMX or C2 audio decoders, whether they are software or hardware-based. Consequently, on the Android platform, a conversion is necessary from the decoder to scaletempo2, where the s16 format must be converted to floatp. If I remember correctly, in the audio output (AO) section, when writing data to the audiotrack, it must be converted back from floatp to s16. This format conversion seems to be quite CPU-intensive, potentially causing scaletempo2 to use nearly 10% more CPU resources than scaletempo, especially noticeable with 5.1 or 7.1 surround sound channels where the load increase is more pronounced. Therefore, as a default , is there a way to support s16 internally like scaletempo does, or would it require significant modifications that might be nearly impossible to implement?

@ruihe774
Copy link
Contributor

I apologize if there might have been some noise. Compared to scaletempo, it appears that scaletempo2 lacks internal support for the af_format_s16 format, which is nearly ubiquitous in the Android platform for OMX or C2 audio decoders, whether they are software or hardware-based. Consequently, on the Android platform, a conversion is necessary from the decoder to scaletempo2, where the s16 format must be converted to floatp. If I remember correctly, in the audio output (AO) section, when writing data to the audiotrack, it must be converted back from floatp to s16. This format conversion seems to be quite CPU-intensive, potentially causing scaletempo2 to use nearly 10% more CPU resources than scaletempo, especially noticeable with 5.1 or 7.1 surround sound channels where the load increase is more pronounced. Therefore, as a default , is there a way to support s16 internally like scaletempo does, or would it require significant modifications that might be nearly impossible to implement?

You can try --audiotrack-pcm-float to avoid the conversion back to s16.

@lcksk
Copy link
Contributor

lcksk commented Apr 19, 2024

@ruihe774
Thanks, this is meaningful to me. However, it still does not address the first half of the issue. The data in s16 format obtained from the Android C2 decoder must go through a converter to be transformed into floatp before it can be used with scaletempo2. Although this conversion is automatic, there is still a significant performance cost.

@ruihe774
Copy link
Contributor

@ruihe774 Thanks, this is meaningful to me. However, it still does not address the first half of the issue. The data in s16 format obtained from the Android C2 decoder must go through a converter to be transformed into floatp before it can be used with scaletempo2. Although this conversion is automatic, there is still a significant performance cost.

I don't think the performance cost is mainly due to format conversion. Scaletempo2 is a much more complex algorithm than scaletempo. Did you try floatp input without format conversion for comparison?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants