| 01:00 |
| 01:01:30 | *** | Saving seen data "./dancer.seen" |
| 01:12:16 | | Quit advcomp2019 (Read error: Connection reset by peer) |
| 01:12:41 | | Join advcomp2019 [0] (~advcomp20@user/advcomp2019) |
| 01:57:26 | | Quit Moriar (Quit: Leaving.) |
| 03:00 |
| 03:01:31 | *** | Saving seen data "./dancer.seen" |
| 04:00 |
| 04:08:03 | | Join lebellium [0] (~lebellium@2a01cb0809965300e5e42a88e6e2c7c5.ipv6.abo.wanadoo.fr) |
| 04:08:49 | | Join ZincAlloy [0] (~Adium@ip5f5abcae.dynamic.kabel-deutschland.de) |
| 05:00 |
| 05:01:34 | *** | No seen item changed, no save performed. |
| 05:56:37 | | Quit f1refly (Read error: Connection reset by peer) |
| 05:59:04 | | Join f1refly [0] (~f1refly@p4fc47709.dip0.t-ipconnect.de) |
| 07:00 |
| 07:01:35 | *** | Saving seen data "./dancer.seen" |
| 07:02:59 | | Quit munkis (Ping timeout: 252 seconds) |
| 07:11:02 | yang | There seem to be different "qualities" of FLAC files? However I read once, that every FLAC file can be losslessly transcoded into .wav to create a CD disc. So, why are then different qualities of FLAC files ? |
| 07:17:05 | gevaerts | You can losslessly transcode between wav and flac at will, but they still have a sample rate, bit depth and number of channels. If you get a 11025Hz 8 bit mono flac file, it transcodes to a 11025Hz 8 bit mono wav file, and that's not what we know as "CD quality" |
| 07:17:37 | gevaerts | Flac does not introduce quality loss, but you can lose quality before encoding... |
| 07:34:38 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 07:39:05 | | Quit dconrad (Ping timeout: 265 seconds) |
| 07:47:27 | | Quit ZincAlloy (Quit: Leaving.) |
| 08:00 |
| 08:40:18 | yang | gevaerts: thanks for your explanation |
| 09:00 |
| 09:01:39 | *** | Saving seen data "./dancer.seen" |
| 09:03:06 | | Quit f1refly (Quit: see ya in hell) |
| 09:15:54 | | Quit akaWolf (Ping timeout: 240 seconds) |
| 09:23:21 | | Join f1refly [0] (~f1refly@p2e50bd38.dip0.t-ipconnect.de) |
| 09:27:34 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 09:48:05 | _bilgus | I'm confused I was under the impression in most cases using a const int for a buffer is perfectly fine |
| 09:49:06 | _bilgus | BUT const int bufsz = TAG_MAXLEN+32; |
| 09:49:06 | _bilgus | char buf[bufsz] is ~900 bytes more code than char buf[TAG_MAXLEN+32]; |
| 10:00 |
| 10:21:31 | dconrad | oh my god I don't believe it |
| 10:22:05 | dconrad | the eros q's play/pause clicking can be fixed by... adding a -2 dc offset? |
| 10:22:16 | dconrad | no way that's true... |
| 10:23:25 | dconrad | and also the whole low volume clicking thing was.... because it wants signed data centered around zero, and we were sending data centered around pcm_factor_unity/2? |
| 10:24:26 | dconrad | ... is that also true for the hosted port I wonder |
| 10:27:01 | | Join akaWolf [0] (~akaWolf@akawolf.org) |
| 10:28:19 | dconrad | oh my god yes it is |
| 10:28:29 | dconrad | speachy, any thoughts on the above ^^ ? |
| 10:31:40 | rb-bluebot | Build Server message: New build round started. Revision 7413442411, 303 builds, 10 clients. |
| 10:32:26 | | Join Moriar [0] (~moriar@107-200-193-159.lightspeed.stlsmo.sbcglobal.net) |
| 10:42:10 | rb-bluebot | Build Server message: Build round completed after 631 seconds. |
| 10:42:13 | rb-bluebot | Build Server message: Revision 7413442411 result: All green |
| 10:49:41 | | Join advcomp2019_ [0] (~advcomp20@user/advcomp2019) |
| 10:51:54 | | Join ZincAlloy [0] (~Adium@ip5f5abcae.dynamic.kabel-deutschland.de) |
| 10:53:26 | | Quit advcomp2019 (Ping timeout: 252 seconds) |
| 10:56:48 | | Quit ZincAlloy (Ping timeout: 276 seconds) |
| 11:00 |
| 11:01:40 | *** | Saving seen data "./dancer.seen" |
| 11:02:19 | | Join ZincAlloy [0] (~Adium@2a02:8108:943f:d824:e5b0:855a:89b0:e425) |
| 11:06:42 | | Quit ZincAlloy (Ping timeout: 240 seconds) |
| 11:10:26 | dconrad | so now that it looks like both the standard and alternate volume scaling methods are viable for the eros q native, I wonder if there is a sound quality difference between them |
| 11:11:30 | dconrad | or if the alternate is better as it future-proofs it if we ever get more bit depth to the dac |
| 11:37:14 | speachy | _bilgus: what do you think about adding a hook to gerrit that rejects patches with bad whitespace? |
| 11:37:44 | speachy | (or at least refuses to submit them) |
| 11:38:51 | _bilgus | sounds good but I don't know how often it really happens |
| 11:42:59 | speachy | dconrad: that's an interesting discovery |
| 11:43:10 | | Quit massiveH (Ping timeout: 265 seconds) |
| 11:43:22 | speachy | I suppose this is really a PCM data format thing.. |
| 11:45:19 | speachy | so you're saying our pcm internals are U16, centered at MAX_U16/2, but the ALSA is expecting S16 centered at 0 |
| 11:45:33 | speachy | and the erosQ (and perhaps other codecs) are the same? |
| 11:46:02 | speachy | in our alsa code we can just as easily request U16/U32 formatting |
| 11:46:12 | speachy | or is this purely an issue with the volume control? |
| 11:47:51 | dconrad | well, it would appear the input to the sample scaling function is signed, centered about 0, and even though the function output is signed, we recenter it about PCM_FACTOR_UNITY/2 |
| 11:48:08 | speachy | ok. that's clearly jacked. |
| 11:48:25 | speachy | wonder what other targets use softvol control. |
| 11:48:27 | dconrad | what alsa wants I don't know, but the dac appears to want signed data ("two's complement", per the datasheet) |
| 11:48:49 | dconrad | so I'm unsure if that's by design or an error |
| 11:49:30 | dconrad | and, if I understand correctly, that means we really only get 8 bits of range before it clips? |
| 11:50:09 | | Quit Moriar (Quit: Leaving.) |
| 12:00 |
| 12:17:56 | speachy | hmm. might improve performance on the hosted targets if we use S24 vs S32 in alsa. |
| 12:19:14 | dconrad | won't that just add an extra processing step to truncate it? or rework the scale factor logic, which is magic to me haha |
| 12:29:49 | dconrad | I just can't believe the play/pause clicking was solved so easily... I was really worried it was an inherent hardware flaw that would be impossible to fix |
| 12:39:04 | | Join ZincAlloy [0] (~Adium@2a02:8108:943f:d824:ac12:1506:ffd6:d74f) |
| 13:00 |
| 13:01:43 | *** | Saving seen data "./dancer.seen" |
| 13:29:11 | dconrad | speachy: by the way, do you have any interest (or maybe you already have...) in trying out the native eros q port? I think mine is the only one it's been tested on |
| 13:38:48 | speachy | yes, but much like the m3k, I've had to put my spare time into other pursuits lately |
| 13:39:21 | speachy | (I have two Qs here, one with the v1.2 and the other with the v1.3 OF) |
| 13:41:00 | dconrad | if you wanted to put the native port on one of those, it's a pretty quick process, and it shouldn't care which version the OF is |
| 13:42:39 | dconrad | only downside is no dual boot yet |
| 13:45:30 | dconrad | (and you'll probably want to apply either g#3633 or g#3623) |
| 13:45:33 | rb-bluebot | Gerrit review #3633 at https://gerrit.rockbox.org/r/c/rockbox/+/3633 : ErosQ Native: Add DC Offset to PCM data by Dana Conrad |
| 13:45:34 | rb-bluebot | Gerrit review #3623 at https://gerrit.rockbox.org/r/c/rockbox/+/3623 : Eros Q Native: Alternate 16-bit volume scaling by Dana Conrad |
| 13:53:41 | speachy | first things first... back outside to deal with more household duties |
| 13:56:21 | dconrad | haha yeah I get that |
| 14:00 |
| 14:25:27 | | Quit dconrad (Remote host closed the connection) |
| 14:29:11 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 14:38:29 | | Quit dconrad (Remote host closed the connection) |
| 14:54:14 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 14:58:32 | | Quit dconrad (Ping timeout: 256 seconds) |
| 15:00 |
| 15:01:45 | *** | Saving seen data "./dancer.seen" |
| 15:45:41 | | Join S|h|a|w|n [0] (~shawn156@user/shawn/x-4432647) |
| 15:47:31 | braewoods | speachy: i do know one thing impacts performance. dmix. i assume it's already disabled but it should be if it isn't. |
| 15:47:57 | braewoods | it adds extra overhead which is pointless for a single audio source setup |
| 15:54:37 | braewoods | oh right. i forgot. rockbox does have one kind of "sound effect" |
| 15:54:49 | braewoods | TTS |
| 16:00 |
| 16:38:19 | | Join Moriar [0] (~moriar@107-200-193-159.lightspeed.stlsmo.sbcglobal.net) |
| 16:39:16 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 16:41:35 | yang | Which iRiver player is this ? https://www.bolha.com/image-w920x690/walkman/garmin-kamero-jvc-kamerasony-fotoaparat-slika-5644523.jpg |
| 17:00 |
| 17:01:46 | *** | No seen item changed, no save performed. |
| 17:14:42 | | Quit lebellium (Ping timeout: 240 seconds) |
| 17:14:47 | | Quit dconrad (Remote host closed the connection) |
| 17:30:31 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 17:34:46 | | Quit dconrad (Ping timeout: 250 seconds) |
| 18:00 |
| 18:00:02 | | Join saratoga [0] (~saratoga@cpe-98-10-205-66.rochester.res.rr.com) |
| 18:00:30 | saratoga | dconrad: PCM_FACTOR_UNITY/2 is equal to 1/2 |
| 18:00:42 | saratoga | adding one half and then truncating is a common way to implement rounding |
| 18:03:42 | saratoga | yeah, looking at your patch you removed the rounding to the nearest integer |
| 18:04:00 | saratoga | if truncation sounds better than rounding you probably have the fixed point format set wrong |
| 18:09:10 | saratoga | should you be editing pcm_sw_volume.c at all? I think you can do whatever format conversion is needed in the individual device driver (or whatever the equivalent is for hosted devices), putting in device-specific workarounds into the general software volume control seems very messy and probably not needed |
| 18:20:19 | | Join cockroach [0] (~blattodea@user/cockroach) |
| 18:49:16 | | Quit ZincAlloy (Quit: Leaving.) |
| 18:50:13 | speachy | won't adding PCM_FACTOR_UNITY/2 screw up rounding since the sample is signed? |
| 18:50:25 | speachy | (I mean, if the value is < 0, you need to subtract UNITY/2, not add it.. |
| 18:51:30 | braewoods | yang: not any supported one. |
| 18:56:25 | _bilgus | speachy thats a good point |
| 18:57:12 | _bilgus | if you did u + val it would word |
| 18:57:19 | _bilgus | work* |
| 19:00 |
| 19:01:48 | *** | Saving seen data "./dancer.seen" |
| 19:07:30 | speachy | dconrad: please try g#3634 |
| 19:07:32 | rb-bluebot | Gerrit review #3634 at https://gerrit.rockbox.org/r/c/rockbox/+/3634 : pcm: Fix incorrect softvol rounding by Solomon Peachy |
| 19:09:11 | speachy | this makes me wonder if we have similar issues lurking elsewhere in our codebase |
| 19:17:46 | | Join dconrad [0] (~dconrad@208.38.228.17) |
| 19:54:18 | dconrad | speachy, unfortunately g#3634 seems to crackle quite a bit |
| 19:54:20 | rb-bluebot | Gerrit review #3634 at https://gerrit.rockbox.org/r/c/rockbox/+/3634 : pcm: Fix incorrect softvol rounding by Solomon Peachy |
| 19:56:37 | dconrad | however, if I swap the signs on the conditional so it's "? -(PCM_F_T)PCM_FACTOR_UNITY/2 : (PCM_F_T)PCM_FACTOR_UNITY" it seems to not crackle/click at all, including starting/stopping/play/pause etc. |
| 19:57:51 | dconrad | unfortunately my DC bias trick doesn't work to get mute to be silent with this patch, it clicks going in/out of mute |
| 19:58:21 | dconrad | my bad on my understanding of what PCM_FACTOR_UNITY is, btw |
| 20:00 |
| 20:00:37 | speachy | it does seem that the rounding that pcm_scale_sample() does is at the root of this issue. |
| 20:00:57 | braewoods | speachy: is this like the different FP rounding methods? |
| 20:01:22 | braewoods | round even/odd, round towards zero |
| 20:01:24 | braewoods | etc |
| 20:01:29 | speachy | perhaps the most correct-est solution here is to just get rid of it altogether. |
| 20:01:50 | speachy | braewoods: well, it's really "truncate vs round" in this case, but yeah. |
| 20:02:04 | braewoods | maybe we could try using code from libsamplerate? |
| 20:02:10 | braewoods | iirc, it was designed for this problem. |
| 20:02:32 | braewoods | assuming the problem is scaling sample rates to fit the hardware rate |
| 20:02:34 | dconrad | in terms of clicking/artifacting, not rounding at all seems to be the best solution, but maybe it's not the best for sound quality, I'm not sure |
| 20:03:21 | speachy | I'd think that "clicking/popping" is part of "sound quality" :D |
| 20:03:28 | dconrad | well, yeah... haha |
| 20:03:31 | braewoods | https://github.com/libsndfile/libsamplerate |
| 20:04:02 | speachy | I think that, if we don't round, we might never actually use +-MAX_INT |
| 20:04:21 | speachy | wait wait. |
| 20:04:46 | speachy | if our fractional value is at MAX_U32, adding .5*UNITY will overflow |
| 20:05:33 | speachy | and since we shift, we'll end up with 0 instead of MAX_S16 |
| 20:06:07 | speachy | (or more accurately, if our frac value is > (MAX_S32 - UNITY/2) |
| 20:07:14 | speachy | the more I think about it, the more it seems that we shouldn't do any rounding. |
| 20:08:54 | speachy | braewoods: does libsamplerate also deal with bitdepths too? |
| 20:09:02 | speachy | but it does seem to be fundamentally floating-point based. |
| 20:09:06 | braewoods | let me look. |
| 20:09:18 | braewoods | it was designed for desktop use, just the best example i could recall. |
| 20:09:43 | dconrad | I don't quite follow why adding PCM_FACTOR_UNITY/2 is considered rounding tbh, but... wouldn't you want to look at the msb or 2 that get shifted out of the end value and round based on that? |
| 20:09:52 | dconrad | maybe that's what it is doing |
| 20:10:18 | dconrad | like rounding a 0.5 up or 0.4 down |
| 20:10:27 | speachy | in the scaling code, PCM_FACTOR_UNITY is basically equivalent to a sample value of "1" |
| 20:11:13 | speachy | so by using UNITY/2 you're adding the equivalent of 0.5, and then doing the divide/shift. |
| 20:11:37 | speachy | (you need to do it before the shift) |
| 20:12:19 | braewoods | speachy: seems it does rely on FP a lot |
| 20:12:33 | braewoods | ok, nevermind then |
| 20:12:55 | braewoods | i just remembered it from all the packaging i did |
| 20:13:28 | speachy | fp-based is great for desktop applications (where you have fast FPUs even before you consider SIMD engines) |
| 20:13:47 | braewoods | yea but most of our targets lack a FPU |
| 20:13:56 | dconrad | so it sounds like that's basically what it is doing, but in a wacky bitwise way |
| 20:14:09 | speachy | but converting a fp algorithm to something integer-based is still as much of an art as a science. |
| 20:15:17 | speachy | (last two $dayjobs involved needing to convert multi-dimentional FP transforms into integer so they could be implemented in silicon) |
| 20:16:05 | speachy | (I wasn't doing that work but when your power budget is a couple of milliwatts, optimization _really_ matters) |
| 20:16:11 | dconrad | ... though the device I'm interested in (erosq) does appear to have a fpu, would it work/be worth the effort to implement? |
| 20:16:41 | dconrad | or "no way we have the cpu resources either way" |
| 20:16:57 | speachy | if we were using the linux port, sure, we get FP for free, but our bare-metal OS isn't FP-aware |
| 20:17:07 | dconrad | ah, I see |
| 20:17:13 | speachy | (we need to store/restore the FP registers as part of task switching) |
| 20:17:39 | braewoods | speachy: could we leverage the GCC fixed point extensions to help out? |
| 20:17:54 | braewoods | it allows you to interact with them normally |
| 20:18:04 | braewoods | compiler handles all the fixed point crapola so you don't need to simulate t |
| 20:18:06 | braewoods | it |
| 20:18:19 | braewoods | not sure if 4.9 is new enough for that |
| 20:18:23 | speachy | I actually started implementing that a while back but given how expensive interrupt handling already is on our ingenic targts, throwing FP into the mix would probably render our audio playback unusable without further rewrites |
| 20:18:56 | speachy | version 2 of that attempt was hacking in just enough FP support so we could use FP codecs. |
| 20:18:58 | braewoods | i thought fixed point just used existing integer registers? |
| 20:19:22 | speachy | with the thinking that we'd never have more than one codec active so there wouldn't be any need to store/restore FPU state |
| 20:19:54 | speachy | braewoods: FP support, not gcc extensions |
| 20:19:59 | braewoods | ok |
| 20:20:12 | braewoods | just an idea i had if we wanted to revamp some code |
| 20:20:49 | braewoods | https://gcc.gnu.org/onlinedocs/gcc/Fixed-Point.html |
| 20:21:09 | speachy | pretty sure gcc 4.9.x supports it, yes |
| 20:21:48 | speachy | but who knows how efficient it is. |
| 20:22:00 | speachy | https://gcc.gnu.org/onlinedocs/gcc-4.9.4/gcc/Fixed-Point.html#Fixed-Point |
| 20:22:12 | speachy | overflow/rounding isn't implemented anyway. :D |
| 20:22:36 | braewoods | so we'd only get basic support |
| 20:23:50 | braewoods | i just know it's a viable substitute for some uses of floating point |
| 20:24:53 | braewoods | FP is too slow to be practical without the hardware support |
| 20:25:04 | dconrad | (if we're revamping code, my vote goes to >16bit volume scaling −−> dac tho, haha) |
| 20:25:30 | braewoods | it won't be me; i don't know enough about audio signal stuff to write good code. |
| 20:25:44 | speachy | dconrad: amen. start at the PCM driver API and work backwards from there |
| 20:26:03 | braewoods | i'm currently busy with my inflate project anyway |
| 20:26:11 | dconrad | it's ungodly complicated, I def understand why nobody's done it yet |
| 20:26:39 | speachy | for sanity's sake we'd probably want to always go with whatever the DAC supports, rather than being able to switch between (eg) 16/24bpp. |
| 20:26:46 | speachy | on the fly, I mean |
| 20:27:16 | braewoods | we could create an abstraction that exposes the audio IO and their capabilities |
| 20:27:26 | braewoods | (i don't know what rockbox currently does so yea |
| 20:27:28 | speachy | we already have that. |
| 20:27:47 | braewoods | ok good stuff |
| 20:27:48 | speachy | it's just that bitdepth is not in the set |
| 20:28:03 | braewoods | because it was always hard coded before? |
| 20:28:08 | speachy | yep |
| 20:28:42 | speachy | high-bitdepth stuff didn't hit the mainstream untill well after the general DAP market had long since been gutted by smartphones |
| 20:28:53 | speachy | (ie well after rockbox's heydey) |
| 20:30:20 | dconrad | if we had a higher depth signal path all the way through, that might actually bring some audiophile types in as users I would think, they love that stuff |
| 20:30:23 | speachy | but anyway. starting at the PCM DAC API, working backwards, lets us do the minimal work to allow for softvol to work without touching most of the core |
| 20:30:39 | speachy | and we can push back incrementally from there |
| 20:31:23 | speachy | but other than FLAC, I don't think any of the codecs would ever care about high-res audio. |
| 20:31:24 | dconrad | I was looking at that this morning, are you talking about like pcm_play_dma_status_callback_int and pcm_play_dma_complete_callback? |
| 20:31:48 | dconrad | I think that's where the driver interfaces with the generic pcm code |
| 20:32:01 | braewoods | speachy: musepack supports up to 32 bit samples, iirc |
| 20:32:02 | speachy | dconrad: that code wouldn't need to change actually −− but the code that fills those buffers would care. |
| 20:32:10 | dconrad | I see |
| 20:32:44 | speachy | I think for sanity's sake we'd always want to send high-res samples to the DAC. |
| 20:33:16 | speachy | eg always operate the erosq at 24bit because making it runtime switchable greatly complicates the implementation |
| 20:33:51 | dconrad | yeah I don't see any real reason to ever want less resolution, power saving maybe |
| 20:34:25 | speachy | power&|performance, but I don't think any of the targets where we're performance limited support high bitdepths anyway |
| 20:35:28 | speachy | after the minimal work to enable softvol, the next step would be to push bitdepth awareness into the DSP core. |
| 20:35:37 | dconrad | supposedly we're probably only 2 bits short as it is, but apparently it makes a difference based on a noticeable difference between hosted and native |
| 20:35:49 | speachy | from there, individual PCM sample sources could be ported/enhanced as needed/convenient |
| 20:36:20 | * | speachy pages MrSomebody. |
| 20:36:38 | dconrad | that might go unanswered haha |
| 20:37:03 | speachy | hmm |
| 20:37:16 | dconrad | but I think I might give it another shot to do the swvol |
| 20:37:27 | speachy | the onlyuser of the "native" PCM API now is the plugin API |
| 20:37:47 | speachy | (well, other than our DSP layer) |
| 20:38:04 | speachy | tbh we should move all of those users onto the DSP API instead. |
| 20:44:22 | | Join massiveH [0] (~massiveH@ool-18e4e82f.dyn.optonline.net) |
| 20:45:50 | dconrad | speachy: so all that said, what's your thoughts on g#3633? |
| 20:45:52 | rb-bluebot | Gerrit review #3633 at https://gerrit.rockbox.org/r/c/rockbox/+/3633 : ErosQ Native: Add DC Offset to PCM data by Dana Conrad |
| 20:46:57 | dconrad | should I go ahead and remove the preexisting rounding? I don't want to break any other targets... |
| 20:47:14 | speachy | IMO the proper solution is to nuke the rounding. it's clearly broken. |
| 20:47:31 | dconrad | I can do that |
| 20:47:40 | speachy | the DC offset shouldn't be needed if the rounding is gone. |
| 20:48:01 | dconrad | I do believe it will click without it, I'll try real quick |
| 20:50:31 | dconrad | yeah, it clicks on play/pause and going in/out of mute |
| 20:50:39 | dconrad | playback is clean though |
| 20:50:55 | dconrad | I think the clicking is a hardware-specific bug |
| 20:51:01 | dconrad | *play/pause/muting |
| 20:51:29 | speachy | does that include tha change here: https://gerrit.rockbox.org/r/c/rockbox/+/3633/4/firmware/drivers/audio/eros_qn_codec.c#64 |
| 20:51:58 | dconrad | yeah, that's the muting |
| 20:52:36 | speachy | is INT_MIN negative, or 0? |
| 20:52:55 | speachy | from the dac's pecspective, mute == 0 |
| 20:53:28 | dconrad | let me try to find that |
| 20:53:28 | speachy | (since we're talking about a signed sample) |
| 20:54:09 | speachy | (PCM_MUTE_LEVEL needs to be 0) |
| 20:54:46 | dconrad | I think PCM_MUTE_LEVEL is really just a signal to tell pcm_centibels_to_factor() to return 0 regardless |
| 20:55:03 | dconrad | which with the DC offset will be dc -1 |
| 20:55:26 | speachy | dc -1 will prevent the DAC from auto-muting. |
| 20:55:39 | dconrad | that's probably why it works to keep it from clicking |
| 20:55:41 | speachy | yeah |
| 20:55:52 | dconrad | though I don't know why +1 doesn't do the same thing |
| 20:56:33 | speachy | so what I suggest is twofold, nuke the rounding first as that's clearly jacked. and in the erosq code, don't specify PCM_MUTE_LEVEL and instead use an explicit -1 or whatever instead. |
| 20:58:14 | dconrad | I can do the first, but the second, how can I ensure that except to go through pcm_set_master_volume()? I don't quite follow |
| 20:58:48 | speachy | I mean, in audiohw_set_volume() in eros_qn_codec.c |
| 20:59:15 | speachy | l = l << PCM5102A_VOLUME_MIN ? -1 : l; |
| 20:59:25 | speachy | oh wait nevermind |
| 20:59:27 | speachy | d'oh |
| 21:00 |
| 21:00:08 | dconrad | yeah, that's gonna get mangled into a multiplicative factor unless it's PCM_MUTE_LEVEL |
| 21:00:27 | speachy | "muted" means "still playing but at 0 volume" |
| 21:00:35 | dconrad | yeah |
| 21:00:35 | speachy | s/0/min/ |
| 21:00:54 | dconrad | I did try using the hardware muting stuff but everything clicks at least a little |
| 21:01:44 | speachy | hmm. I wonder what the XSMT pin on the codec is doing. |
| 21:01:49 | *** | No seen item changed, no save performed. |
| 21:01:50 | dconrad | which is really funny, all the datasheets say "this part is POP AND CLICKLESS" and like... no it's not haha |
| 21:02:06 | dconrad | yeah, I tried that earlier - it works, but it clicks |
| 21:02:17 | speachy | "when the device detects continuous zero data, it enters a full analog mute condition" |
| 21:02:33 | dconrad | I can give it a try with the DC offset, I don't know if I tried that |
| 21:02:58 | dconrad | or just try it again without just to double check |
| 21:03:11 | speachy | you are controlling that pin? |
| 21:03:15 | dconrad | yeah |
| 21:04:23 | dconrad | well, there's auto-mute and the XMIT pin, I think they're separate |
| 21:04:42 | dconrad | auto-mute, I think means it detected that the clock stopped |
| 21:05:00 | speachy | clock stopped OR the sample data is all zeros for a certian amount of time |
| 21:05:11 | | Join munkis [0] (~mendel_mu@ool-ae2cb218.dyn.optonline.net) |
| 21:05:15 | dconrad | aha, that's probably the fundamental issue right there |
| 21:05:24 | speachy | that's why the DC offset works |
| 21:05:31 | speachy | prevents the codec from muting. |
| 21:05:46 | dconrad | though I do wonder why -1 works and +1 doesn't, magic probably |
| 21:05:54 | speachy | btw, the +1 doesn't work there because it's shifted away, whereas the negative value remains negative due to the lack of rounding |
| 21:06:04 | dconrad | oh, well there ya go |
| 21:07:26 | dconrad | would you want removing the rounding to be its own commit? |
| 21:07:28 | speachy | so perhaps instead, in pcm_hw_set_vol() you can assert the XSMT instead? |
| 21:07:38 | speachy | if the volume is min, I mean |
| 21:07:54 | dconrad | I can try, but unfortunately do think it's going to be audible |
| 21:08:41 | speachy | and yes, remove rounding in a separate commit, or I can do it instead. |
| 21:08:48 | dconrad | wait, instead of the DC offset you mean? or the PCM_MUTE_LEVEL, or both |
| 21:09:00 | speachy | yes, instead of both |
| 21:09:10 | dconrad | gotcha, I'll try it |
| 21:09:18 | speachy | (with the rounding removed) |
| 21:09:23 | dconrad | right |
| 21:14:48 | dconrad | ok, so with rounding removed, no DC bias, and using the XMIT pin as muting, we have: |
| 21:15:44 | dconrad | playback is clean, muting/unmuting clicks, any start/stop/play/pause or any operation that starts/stops pcm data clicks |
| 21:15:59 | * | speachy frowns. |
| 21:16:33 | speachy | okay, what happens if you toggle the XSMT when volume is set to "normal" levels? |
| 21:16:51 | dconrad | hm, I'm not sure how to accomplish that |
| 21:17:22 | dconrad | any easy places to plop it in come to mind? |
| 21:17:23 | speachy | hack something into the button code |
| 21:17:29 | speachy | eg press M for mute. :D |
| 21:17:51 | speachy | (ie in the erosq's button code, before it gets sent to the rockbox core) |
| 21:18:03 | dconrad | ah, I'll give it a try |
| 21:18:05 | speachy | I used the lock switch on the X3 to accomplish similar stuff |
| 21:22:14 | speachy | braewoods: btw, this was my WIP FPU code: g#2814 |
| 21:22:16 | rb-bluebot | Gerrit review #2814 at https://gerrit.rockbox.org/r/c/rockbox/+/2814 : WIP: Native MIPS supports FPU. (DO NOT MERGE) by Solomon Peachy |
| 21:24:11 | dconrad | ok, so I used vol+/- to mute/unmute at -45dB |
| 21:24:47 | dconrad | you can still hear the click a little, though it doesn't seem to scale up with volume so it's buried a little |
| 21:24:55 | dconrad | though I'm not sure what that proves tbh |
| 21:25:10 | speachy | it proves it's a hw artifact. |
| 21:26:19 | speachy | oh, are you using HP or LO out? |
| 21:26:23 | dconrad | headphone |
| 21:26:41 | dconrad | I suppose I can go try LO out, I think I tried that earlier as well |
| 21:26:49 | speachy | the OF disables the HP and/or LO output altogether |
| 21:27:09 | speachy | if you mute, you switch outputs to something that's not connected |
| 21:27:15 | speachy | then you don't get a pop |
| 21:27:34 | speachy | from the codec being special |
| 21:27:52 | dconrad | hm, I'm pretty sure I ohm'd out the connections on the LO jack direct to the dac |
| 21:28:39 | speachy | might be worth trying this via LO instead of HP, to eliminate that as the source of the click |
| 21:28:52 | dconrad | yeah I can try that |
| 21:28:55 | dconrad | one second |
| 21:29:54 | speachy | the HP amp presumably has a mute too? |
| 21:31:22 | dconrad | ok, back |
| 21:32:10 | dconrad | the click on play/pause/mute are all there on the LO out, though I had to turn the volume way down manually to hear it clearly, no surprise there |
| 21:32:14 | dconrad | but, it's present |
| 21:32:22 | dconrad | yeah the HP amp has a mute as well |
| 21:32:32 | dconrad | I as does the stereo switch |
| 21:32:53 | speachy | so mute the switch? :D |
| 21:33:16 | speachy | (and the HP amp should be muted too, if there are no headphones inserted..) |
| 21:33:49 | dconrad | in terms of least to most loud click, it goes HP power = switch mute = dac xmit = switch input sel, then hp mute, then dac analog power |
| 21:34:09 | dconrad | (I tested all those earlier) |
| 21:34:18 | dconrad | but... all click |
| 21:34:34 | dconrad | (at least a little) |
| 21:36:49 | dconrad | the dac analog power is a proper pop, which I guess shouldn't be surprising |
| 21:38:10 | dconrad | I suppose I can test the hp amp and switch mutes again just for fun |
| 21:38:20 | dconrad | what with rounding removed |
| 21:39:06 | dconrad | though that wouldn't remove the pcm data start/stop clicking, especially when using the LO |
| 21:39:33 | * | speachy nods. |
| 21:40:05 | speachy | well, adding in HP muting is a good idea regardless (if only to save power when no HP is present) |
| 21:41:03 | dconrad | yeah I can see what happens if I power down the HP amp when headphones are detected as removed |
| 21:41:16 | dconrad | hopefully it doesn't cause an audible artifact on the LO |
| 21:41:42 | dconrad | or set it to mute, since there won't be anything to hear its click anyway |
| 21:47:36 | dconrad | ok, so that got messy but I think I cleaned it up |
| 21:48:21 | dconrad | so I'm going to try this with DC offset, with PCM_MUTE_LEVEL (though that shouldn't affect anything) |
| 21:48:44 | dconrad | and no rounding obvs |
| 21:55:41 | dconrad | ok, there don't seem to be any weird side effects of muting the headphone amp when the headphones are inserted/removed |
| 21:56:02 | dconrad | the LO seems to be completely unaffected |
| 22:00 |
| 22:10:29 | dconrad | g#3635, simple enough |
| 22:10:31 | rb-bluebot | Gerrit review #3635 at https://gerrit.rockbox.org/r/c/rockbox/+/3635 : Software Volume: Remove sample rounding by Dana Conrad |
| 22:11:01 | dconrad | and now that we've eliminated all other options, g#3633 is updated |
| 22:11:03 | rb-bluebot | Gerrit review #3633 at https://gerrit.rockbox.org/r/c/rockbox/+/3633 : ErosQ Native: Add DC Offset to PCM data by Dana Conrad |
| 22:18:26 | rb-bluebot | Build Server message: New build round started. Revision ecf0d631e7, 303 builds, 9 clients. |
| 22:20:13 | speachy | dconrad: you didn't update 3633 |
| 22:20:38 | dconrad | ?? oh, I will need to fix the merge conflict |
| 22:20:50 | dconrad | won't take but a second |
| 22:24:15 | dconrad | ok, merge conflict resolved |
| 22:24:47 | | Quit cockroach (Ping timeout: 252 seconds) |
| 22:29:31 | rb-bluebot | Build Server message: Build round completed after 665 seconds. |
| 22:29:34 | rb-bluebot | Build Server message: Revision ecf0d631e7 result: All green |
| 22:29:38 | rb-bluebot | Build Server message: New build round started. Revision a2fd00cf38, 303 builds, 9 clients. |
| 22:39:44 | rb-bluebot | Build Server message: Build round completed after 606 seconds. |
| 22:39:46 | rb-bluebot | Build Server message: Revision a2fd00cf38 result: All green |
| 23:00 |
| 23:01:10 | | Quit skipwich (Read error: Connection reset by peer) |
| 23:01:51 | *** | Saving seen data "./dancer.seen" |
| 23:54:01 | | Quit dconrad (Remote host closed the connection) |
icon identifies that the person is a core developer (has commit access).