dmrschmidt / DSWaveformImage

Generate waveform images from audio files on iOS, macOS & visionOS in Swift. Native SwiftUI & UIKit views.
MIT License
1.04k stars 113 forks source link

Waveform disappears when you reopen the app #48

Closed yspreen closed 1 year ago

yspreen commented 1 year ago

If I have a waveform image shown when backgrounding the app, the view will be empty when I re-foreground the app.
my theory is that this might be because of storage access getting blocked when the app backgrounds? the audio URL is local, but the library logs ERROR loading asset / audio track upon backgrounding. if I scroll the view out of the viewframe and back in, then the audio wave is back. the file is never touched, it's always on disk and shouldn't lead to an error like this. I'm also not sending new URLs to the wave view, I checked that

dmrschmidt commented 1 year ago

Hey @yspreen,

The first thing I'd suggest to investigate the root cause for this, is updating to the latest version (11.0.0). The required changes should be trivial. What this will do in your case, is improve error messaging. If this will not be sufficient to find the root cause, please let me know what the exact error is here. And some more details will help, backgrounding the app should not affect the view at all, so a little bit about the lifecycle flow could be helpful.

yspreen commented 1 year ago

I just realized that I was still on the old version number 9. For some reason Xcode will default to version 9 instead of 11 when you’re adding the package. It’s fixed now. Now I only have this clipping issue where the bar seems to be cut off at the top and the bottom for very high volume segments. I’ll investigate that next.

On Sat, Nov 12, 2022 at 3:55 PM Dennis Schmidt @.***> wrote:

Hey @yspreen https://github.com/yspreen,

The first thing I'd suggest to investigate the root cause for this, is updating to the latest version (11.0.0). The required changes https://github.com/dmrschmidt/DSWaveformImage#migration should be trivial. What this will do in your case, is improve error messaging https://github.com/dmrschmidt/DSWaveformImage/commit/d2f939e99c545a3f2d8facfd84a30938a64b46f8. If this will not be sufficient to find the root cause, please let me know what the exact error is here. And some more details will help, backgrounding the app should not affect the view at all, so a little bit about the lifecycle flow could be helpful.

— Reply to this email directly, view it on GitHub https://github.com/dmrschmidt/DSWaveformImage/issues/48#issuecomment-1312570091, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAL3ZYWT5J5JTC3KEQ5YZDWH774JANCNFSM6AAAAAAR6PJZZI . You are receiving this because you were mentioned.Message ID: @.***>

yspreen commented 1 year ago
image

the left one is cut off, and the highest bars are both cut off... maybe I should open a new ticket

dmrschmidt commented 1 year ago

huh, I’ll have a look why Xcode may pick that version instead of the latest. Thanks for letting me know.

Im not on my laptop anymore right now to double check, but there may be some clipping happening for values over 0dB. If you could share an audio file here that has this issue I could take a look tomorrow or after, if the reason doesn’t lay somewhere else. On 12. Nov 2022 at 21:57 +0100, Yannick Spreen @.***>, wrote:

I just realized that I was still on the old version number 9. For some reason Xcode will default to version 9 instead of 11 when you’re adding the package. It’s fixed now. Now I only have this clipping issue where the bar seems to be cut off at the top and the bottom for very high volume segments. I’ll investigate that next.

On Sat, Nov 12, 2022 at 3:55 PM Dennis Schmidt @.***> wrote:

Hey @yspreen https://github.com/yspreen,

The first thing I'd suggest to investigate the root cause for this, is updating to the latest version (11.0.0). The required changes https://github.com/dmrschmidt/DSWaveformImage#migration should be trivial. What this will do in your case, is improve error messaging https://github.com/dmrschmidt/DSWaveformImage/commit/d2f939e99c545a3f2d8facfd84a30938a64b46f8. If this will not be sufficient to find the root cause, please let me know what the exact error is here. And some more details will help, backgrounding the app should not affect the view at all, so a little bit about the lifecycle flow could be helpful.

— Reply to this email directly, view it on GitHub https://github.com/dmrschmidt/DSWaveformImage/issues/48#issuecomment-1312570091, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAL3ZYWT5J5JTC3KEQ5YZDWH774JANCNFSM6AAAAAAR6PJZZI . You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

yspreen commented 1 year ago

this is the audio: https://we.tl/t-73qAXVRbxy

@dmrschmidt

dmrschmidt commented 1 year ago

Hmm… So unfortunately I won’t be able to debug this properly today and only had a listen to the audio file. As it’s a normal mic recording, there shouldn’t be any vertical clipping. So I’m wondering whether the view view itself is clipped by some containing view? If not, have you tried setting the verticalScalingFactor to a lower value? (Shouldn’t be necessary, but will be interesting to know whether it still clips at, say, 0.5)

And to just to be sure. You are drawing the recording from the file itself, and not from raw mic output where you do the dB conversion manually, correct?

And is this UIKit or SwiftUI you’re using? Could you post a screenshot of the view from the view debugger here maybe? Ideally with it highlighted and the inspector open on the right hand side to see it’s properties. On 12. Nov 2022 at 22:14 +0100, Yannick Spreen @.***>, wrote:

this is the audio: https://we.tl/t-73qAXVRbxy @dmrschmidt — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yspreen commented 1 year ago

i’ll make sure to create a swift project for reproduction of this bug tonight ✌️

On Sat, Nov 12, 2022 at 11:58 PM Dennis Schmidt @.***> wrote:

Hmm… So unfortunately I won’t be able to debug this properly today and only had a listen to the audio file. As it’s a normal mic recording, there shouldn’t be any vertical clipping. So I’m wondering whether the view view itself is clipped by some containing view? If not, have you tried setting the verticalScalingFactor to a lower value? (Shouldn’t be necessary, but will be interesting to know whether it still clips at, say, 0.5)

And to just to be sure. You are drawing the recording from the file itself, and not from raw mic output where you do the dB conversion manually, correct?

And is this UIKit or SwiftUI you’re using? Could you post a screenshot of the view from the view debugger here maybe? Ideally with it highlighted and the inspector open on the right hand side to see it’s properties. On 12. Nov 2022 at 22:14 +0100, Yannick Spreen @.***>, wrote:

this is the audio: https://we.tl/t-73qAXVRbxy @dmrschmidt — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/dmrschmidt/DSWaveformImage/issues/48#issuecomment-1312639835, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAL3Z7KH7LE7CGT6KQPL2TWIBYQTANCNFSM6AAAAAAR6PJZZI . You are receiving this because you were mentioned.Message ID: @.***>

yspreen commented 1 year ago

@dmrschmidt https://github.com/yspreen/waveform-test

yspreen commented 1 year ago

I guess loudestClipValue is hard coded to 0.0dB. Would make sense to do that dynamically?

dmrschmidt commented 1 year ago

Alright, I've found the culprit. loudestClipValue isn't the cause. I could still consider exposing this publicly though in case anyone runs into use cases where adjusting this would be desired. In my understanding 0dB should be the loudest possible value of a recording.

The problem instead is mapping what "maximum amplitude" means for different center positions. Currently, maximum is simply the total height of the view. Which means that top and bottom positioned views render correctly. For anything in between verticalScalingFactor needs to be adjusted.

I had felt that this is a decent compromise. But it wasn't obvious enough in this case now and even myself, I didn't remember. So I'm wondering how this could be improved.

I don't think the current solution is wrong. But it's apparently also not obvious enough.

One option I considered was to adjust this factor dynamically, to ensure the view always fits inside the bounds by squeezing it. But this would be increasingly useless the closer to the border the center is.

Another option I guess could be to draw outside the view bounds. Even then, the issue remains though, that intuitively, a verticalScalingFactor of 1 should behave differently when drawing a view at the top vs in the center. I'd assume 1 always means "use / draw in the full available space.

Thoughts?

dmrschmidt commented 1 year ago
image

the left one is cut off, and the highest bars are both cut off... maybe I should open a new ticket

The horizontal clipping should be fixed now in a2ab645, released as 11.0.1.

yspreen commented 1 year ago

speaking about the first bar only right?

On Tue, Nov 15, 2022 at 2:59 AM Dennis Schmidt @.***> wrote:

[image: image] https://user-images.githubusercontent.com/12631527/201494646-6f6dad70-7513-4d44-95ea-5e8607bfcc71.png

the left one is cut off, and the highest bars are both cut off... maybe I should open a new ticket

This should be fixed now in a2ab645 https://github.com/dmrschmidt/DSWaveformImage/commit/a2ab6459fa6b294f566104ccc769c33f516b6d4b, released as 11.0.1.

— Reply to this email directly, view it on GitHub https://github.com/dmrschmidt/DSWaveformImage/issues/48#issuecomment-1314925514, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADAL3Z3X4ALRIX62WKWVDCLWIM7EPANCNFSM6AAAAAAR6PJZZI . You are receiving this because you were mentioned.Message ID: @.***>

dmrschmidt commented 1 year ago

Yes, the first bar is now always fully visible.

The vertical clipping doesn't need a fix in the library necessarily. How you tweaked it in the test project (setting verticalScalingFactor to a value < 0.5 is currently the expected usage.

I do acknowledge though, that this is not necessarily the best developer experience yet. Hence my thinking out loud in https://github.com/dmrschmidt/DSWaveformImage/issues/48#issuecomment-1314812394. I'd be really interested in your thoughts here. Is it just documentation that needs improvement? What was your expected behavior? etc

yspreen commented 1 year ago

well I'd expect the highest bar is always at 100% height. and it scales according to the max volume of the clip. now everything except that one clip looks kinda quiet

yspreen commented 1 year ago

thank you for fixing the first bar! works perfectly now

yspreen commented 1 year ago

okay I just saw that comment. I'm confused. so 0.5 just means 1.0 because a bar is filling half the view not all of it?

dmrschmidt commented 1 year ago

Essentially yes,

If position just allowed either fully top or straight in the middle, it would be more intuitive if the scaling factor reference changed.

But since the center can be anywhere, it’s not quite so obvious anymore, so 1 always means full view height and 0.5 is hence always half the view weight.

On 15. Nov 2022 at 16:53 +0100, Yannick Spreen @.***>, wrote:

okay I just saw that comment. I'm confused. so 0.5 just means 1.0 because a bar is filling half the view not all of it? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

yspreen commented 1 year ago

got it. thanks for explaining! the only open question is: will the loudest value always be a full bar (at scaling 0.5)?
as I understand it, currently the scale is to 0db and not to the loudest value in the recording. would be cool if the scale is always fully used!

dmrschmidt commented 1 year ago

Yes, the loudest value at 0dB will always be a full bar. This is so far not configurable simply because a) I've oriented myself along how other audio apps are doing it, and they all scale relative to 0dB b) I hadn't found a performant way to get the maximum value using Accelerate

It is determined later in the drawing cycle, but not actually used for scaling, only for a better gradient. The performance impact may be low, but I want to keep performance reasonable and its already noticeable when drawing multiple waveforms at once, say in a list of music.

I will add this to the wish list though.

dmrschmidt commented 1 year ago

I think this can be closed now. Please feel free to re-open.

yspreen commented 1 year ago

absolutely! thanks for the support