Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Add ability to control resampling quality for SoftAE from settings #2679

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
9 participants
Member

Karlson2k commented May 1, 2013

Useful both for limited hardware (allow to lower CPU usage) and for powerful hardware (allow to set better quality).

Member

FernetMenta commented May 1, 2013

surgery on an almost dead patient :)
libsamplerate can't handle planar audio formats and I am working on an audio engine which uses ffmpeg instead.

If this setting is really required, it should only show up if SoftAE is used. Including samplerate.h in GuiSettings is really ugly.

Owner

MartijnKaijser commented May 1, 2013

also the whole settings handling is rewritten

Member

fritsch commented May 1, 2013

I am not totally against such a change, but make sure in the default method, that the hardcoded 1 will always be something valid, e.g. MEDIUM_QUALITY whatever which can change when the header changes.

Btw. for all resampling enthusiasts: http://src.infinitewave.ca/ <- have a look here.

Contributor

aballier commented May 1, 2013

@fernetmenta I'm not sure a FFmpeg resampler is the best idea, it is nice as a fallback to handle FFmpeg formats as the library garantees you it can handle them all, but for AE I'd rather go for a libsoxr resampler ( http://sourceforge.net/p/soxr/wiki/Home/ , very similar API to libswr anyway). FFmpeg libswr is fast but not so good quality, libsamplerate is very good quality but very slow, libsoxr is still good quality but also fast. See: http://sourceforge.net/mailarchive/message.php?msg_id=30375103 (audacity switched to libsoxr)

Btw, when I started to work on sth like that I was told noone wants the core of AE to depend on FFmpeg... not sure if that changed.

Member

fritsch commented May 1, 2013

@aballier: See the above link. soxr can now be made part of ffmpeg http://ffmpeg.org/trac/ffmpeg/wiki/FFmpeg%20and%20the%20SoX%20Resampler

Also see the comment here: http://transcoding.wordpress.com/2011/11/16/careful-with-audio-resampling-using-ffmpeg/

Btw the ffmpeg resampler is already used when dvdplayer finds a "non supported format" right now.

Member

fritsch commented May 1, 2013

In my oppinion it is a whole lot of work to implement separately in AE what ffmpeg already does "for free". Which is resample (soxr can be used, if needed), remapping, decode, encode.

Additionally we get some things for free, that we don't have now. Proper delay measurement or no need to decode to float and transcode to something else, as this can go by one step if needed.

Contributor

aballier commented May 1, 2013

@fritsch yes i know soxr can be used through libswr, but why another layer when we can use it directly ? :) esp. when it seems some people are trying to make ffmpeg optional. And I know libswr is used as a fallback since I added this fallback, but IMHO its better to keep this as a fallback because of its supposedly lower quality.

I fully agree that reinventing the wheel in AE is not a good idea, I'm just suggesting soxr is likely a better choice as a wheel.

Member

fritsch commented May 1, 2013

@aballier:
Those people should stand up ;-) From what I see in the code we cannot play a single video without ffmpeg. The whole way as the (especially video) architecture works is ffmpeg bound. The current implementation with the libsamplerate, that is running in Medium Quality by default is nothing that does a whole lot into a quality direction ...

As the above link mentions: Working with a resampler is nothing one does with "just plugging in" and using the standard parameters and all is fine. Perhaps the defaults for soxr are always failsafe.

Directly using in that case means: decoding we must to an intermittant format, be it float, planar float or whatever. So every time ffmpeg changes, we need to consider changing our resampler? no, that is not the way it should be.

Why do we have to go out of ffmpeg? We can stick in there and do all our stuff without having to convert back and forth as we do now - including managing our buffers, error prone channel remapping and so on.

Another pro argument: Next time ffmpeg bumps, we are not f__ed the same way as now - cause even AE doing a whole lot separately does the key things within ffmpeg (Encoding to AC3 or DTS).

Feature wise, I personally don't see anything in AE (decode, transcode, mapping), that we could not get directly out of ffmpeg. There is also the possibility to implement it that way that a whole lot of filters than can interact with ffmpeg directly can be used additionally.

Contributor

aballier commented May 1, 2013

@fritsch see #1290

you dont need to change the resampler because there are (or should be) libswr fallbacks when needed; it just becomes a bit less efficient (decode - resample - send to AE - resample - send to sound card) vs (decode - send to AE - resample - send to soundcard); there is also no problem with next time ffmpeg changes with these fallbacks.

if AE supports planar formats, then eg AC3 encoder can just ask for its planar format and AE will feed it with it; if the sounds sent to AE are in another format, then AE will resample them before sending to the encoder in the correct format; I don't see anything that is ffmpeg-specific there, AE could resample with libsoxr, libswr, src, or anything else, it'd be the same.

Member

fritsch commented May 1, 2013

If you are sitting in a house called AE - that argumentation is totally valid, als from a technical point of view. If you think about constructing a new road, perhaps one could drive around :-)

Member

Karlson2k commented May 1, 2013

SoftAE is used on all platforms.
How can I detect (filter) usage of SoftAE?

Quality control can be used regardless of resampler type. Just map from XBMC setting to resampler setting. All know resamplers have such kind of setting.

Contributor

davilla commented May 1, 2013

SoftAE is NOT used on all platforms :)

Member

Karlson2k commented May 1, 2013

@davilla, OK, didn't really check makefiles, but SoftAE code is designed for Win32, Darwin, BSD, Linux, RaspberryPi and Android. Just give me some hit, how to filter needed platforms.

Contributor

davilla commented May 1, 2013

SoftAE is not designed to work with darwin, see xbmc/cores/AudioEngine/Engines/* and realize that SoftAE is an AudioEngine 'Engine' of which there are three. CoreAudio, PulseAE and SoftAE. Currently, Darwin uses CoreAudio and linux can use PulseAE or SoftAE.

Member

Karlson2k commented May 1, 2013

Nice, I know about different engines.
OK, so I can filter darwin with ifdefs, but how to deal with linux? Linux engine selection is done at compile time or at run time?

Member

fritsch commented May 1, 2013

Currently it is a nice #ifdef thingy, where SoftAE is kind of a fallback.

Have a look in the AEFactory.cpp - the mess starts exactly there.

Member

FernetMenta commented May 2, 2013

AE_QUALITY_INSANE = 100
that fits to the design of SoftAE, how about AE_QUALITY_ISSUE = -1000000 :)

@aballier
It's not only resampling, that can be done in an extra stage behind all that convert and remap mess currently in AE. That engine has bothered me for quite a while: on every commercial break I lose 2 secs of audio and video stutters. Lack of planar audio formats just increased frustration.

Contributor

davilla commented May 3, 2013

ok, we seemed to have a feeling that a big refactor is needed. How/when do we start. I'm game.

Member

FernetMenta commented May 3, 2013

I have already started a new engine: FernetMenta/xbmc@4f675e4
This can run in parallel to SoftAE.

Contributor

davilla commented May 3, 2013

nice, as soon as this has any degree of stability, it should get injected with configure/compile switch.

Contributor

jenkins4xbmc commented Jun 16, 2013

Is this PR ment to be tested by jenkins?

Owner

MartijnKaijser commented Jul 23, 2013

@fritsch
I guess this PR will become moot very soon?

Member

Karlson2k commented Jul 23, 2013

It's based on old settings system, but can be updated.

Member

fritsch commented Jul 23, 2013

ActiveAE is currently PRd as an alternative to SoftAE not as replacement yet. ActiveAE won't use the same resampler. We use internal ffmpeg resampler, which can be - if someone is concerned about quality also the new shiny sox resampler. There is work ongoing to make that actually happen.

Btw. i don't want to start a quality war right now. SoftAE always used MEDIUM_QUALITY cause of performance reasons - so the resampler discussion was not so "audiophile" as in 100% vs. 95% :-)

Member

Karlson2k commented Jul 24, 2013

@fritsch So it's better to implement quality settings for SoftAE (at "Expert" level) and then support same quality settings in ActiveAE? :)

Contributor

DDDamian commented Jul 24, 2013

It makes sense with any code used. There's a huge difference in the quality levels of every resampler and thus the scaleability for different hardware capabilities. This is a perfect example of why settings exist. This option should be available to any advanced user regardless of engine or resampler.

Member

FernetMenta commented Jul 25, 2013

Quality settings make sense but not if tailored to a resampler which is going to die. Sox uses a different type of setting for this.

Member

fritsch commented Jul 25, 2013

Something fuzzy like Low, Medium, High :-) makes sense, how those are internally mapped, can be decided within the resampler / Engine itself, by acquiring those Gui Settings or ignoring them and mapping them accordingly.

So your patch looks good from this intention, as you introduced an AE Quality enum. Remember, when exposing such settings via the GUI, don't give the user too much options.

Member

FernetMenta commented Jul 25, 2013

Not all engines support this setting. Hence AE needs to be queried for this capability, then set a condition to settings.

Owner

MartijnKaijser commented Jul 25, 2013

Keep it at three settings max if we going to do this.

Contributor

t-nelson commented Jul 25, 2013

Fastest, standard, Best? Standard being good for average desktop class HW,
fastest for embedded, "audiophiles" can spend $10k on their cryogenically
treated treated mains and ethernet cables so I'm guessing they already have
an i7 for their HTPC.

Member

fritsch commented Jul 25, 2013

Yeah I currently follow a similar way. LOW, DEFAULT, HIGH, via: fritsch/xbmc@49b39a0

Member

FernetMenta commented Jul 26, 2013

@Karlson2k do you mind if I do the settings and query of capability in #2970? ActiveAE is prepared for changing resample quality while playing a stream. I would like to test this.

Member

Karlson2k commented Jul 26, 2013

@fernetmenta Let me publish a template with settings, than use it in engine. 1 or 2 days.

Member

Karlson2k commented Jul 28, 2013

@fernetmenta @fritsch
Your can use https://github.com/Karlson2k/xbmc/compare/set_resample_q02?expand=1 as template for settings, Just cherrypick commits from this branch.
You need to override 'IsQualityLevelSupported' in your AE, no need to override 'IsQualitySettingSupported'.

Member

FernetMenta commented Jul 29, 2013

Thanks, some notes:

  • I don't think we need 6 different settings for quality. The majority has voted for 3.
  • IsQualitySettingSupported should be in-line with SupportsRaw and SupportsDrain I already have implemented: FernetMenta/xbmc@ca1eb43
  • The setting needs to be registered with for OnSettingsChanged: FernetMenta/xbmc@321beb7
Owner

MartijnKaijser commented Jul 29, 2013

also don't like the used variable names. use what @fritsch suggested: LOW, DEFAULT, HIGH

Member

Karlson2k commented Jul 29, 2013

@fernetmenta

  • Yes, I agree that we don't need 6 quality levels. That's why 3 of them are optional. Engine can support as many levels as needed, it will be reflected in GUI. I believe, that there is demand for something like 'Paranoid' level. Other levels just future-proof. This design is more flexible as it's allow for some Engine (HW-based) to have only 2 levels, for example.
  • Thanks, names should be in-line. I'll rename.
  • Thanks again.

@MartijnKaijser "LOW" and "HIGH" are nice, but "DEFAULT" is better to use for default level. This allow to some Engine (for low power CPUs) to have "LOW" level as default.

Owner

MartijnKaijser commented Jul 29, 2013

make it "MEDIUM" then you can default through setting to either of the settings. remove the rest of the options because why would we have already 3 optional values which won't be used by us

Member

FernetMenta commented Jul 29, 2013

STANDARD is better than DEFAULT for this case. I like the flexible design which allows adding more levels but I would prefer adding them when actually used by an engine.

EDIT: MEDIUM is fine as well

Member

Karlson2k commented Jul 29, 2013

Nice, STANDARD is already in https://github.com/Karlson2k/xbmc/compare/set_resample_q02?expand=1 :)
Branch is updated.

Member

Karlson2k commented Jul 29, 2013

OK, if you don't like new implementation and think that's better to go without reserved levels, I'll remove it.
I don't insist. :)

Contributor

t-nelson commented Jul 29, 2013

How about RED, BLUE, GREEN? :)

Whatever you choose for the names, make sure the setting help message
implies the CPU cost is inversely proportionate to quality.

Member

FernetMenta commented Jul 29, 2013

@Karlson2k could please follow t-nelson's recommendation of stating higher CPU load in the help message. you know how users are: choosing best and then complain about video stuttering or audio drop-outs.
if no one complains about the current version of https://github.com/Karlson2k/xbmc/compare/set_resample_q02?expand=1 , I'll pick it to sledgehammer branch tomorrow. Thanks.

Member

FernetMenta commented Jul 31, 2013

The settings were adopted by #2970 thanks to @Karlson2k

Member

fritsch commented Aug 1, 2013

I still ask myself why real audiophiles resample in the first place.
Technically there is a big difference between up and downsampling,
especially when the div between those is large, e.g. > 2.0
Softae loop has an issue anyways, when the resampler needs too much time.

When introducing so many levels we need to see some figures. Fourrier
spectrum, CPU load when doing average, high res flac and so on. I highly
oriented with shannon nyqhist when choosing active ae numbers and the
longer the filter the easier e.g. better to find / calculate sampling
points - nothing more. Most referenced sites have a db scale anyways.
Thinking logarithmic is highly not linear :-)
Am 31.07.2013 17:34 schrieb "Rainer Hochecker" notifications@github.com:

The settings were adopted by #2970 thanks to @Karlson2k


Reply to this email directly or view it on GitHub.

Owner

MartijnKaijser commented Sep 19, 2013

Already added

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