OxygenCobalt / Auxio

A simple, rational music player for android
GNU General Public License v3.0
2.18k stars 147 forks source link

Warn before adding duplicate songs in playlists #799

Closed nitinsooni closed 5 months ago

nitinsooni commented 5 months ago

Description

Self-explanatory

Problem solved

No response

Other implementations

...

Benefit

...

Duplicates

OxygenCobalt commented 5 months ago

I'm not going to disallow it, but I will likely warn against it. I would rather be more liberal with how playlists can be used.

nitinsooni commented 5 months ago

Yeah warning would be fine. Keep it open for warning feature.

OxygenCobalt commented 5 months ago

That already exists at #747

nitinsooni commented 5 months ago

Oh okay, i searched for 'duplicate' word. Btw, did you face this error? Now it's just coming like every alternate day for me.

Screenshot_20240609-003944

OxygenCobalt commented 5 months ago

I need the stack trace to figure out whats going on there @nitinsooni, please click the more button and copy the information to here.

nitinsooni commented 5 months ago
android.app.ForegroundServiceStartNotAllowedException: Service.startForeground() not allowed due to mAllowStartForeground false: service org.oxycblt.auxio/.music.system.IndexerService
    at android.app.ForegroundServiceStartNotAllowedException$1.createFromParcel(ForegroundServiceStartNotAllowedException.java:54)
    at android.app.ForegroundServiceStartNotAllowedException$1.createFromParcel(ForegroundServiceStartNotAllowedException.java:50)
    at android.os.Parcel.readParcelableInternal(Parcel.java:5015)
    at android.os.Parcel.readParcelable(Parcel.java:4997)
    at android.os.Parcel.createExceptionOrNull(Parcel.java:3177)
    at android.os.Parcel.createException(Parcel.java:3166)
    at android.os.Parcel.readException(Parcel.java:3149)
    at android.os.Parcel.readException(Parcel.java:3091)
    at android.app.IActivityManager$Stub$Proxy.setServiceForeground(IActivityManager.java:6896)
    at android.app.Service.startForeground(Service.java:775)
    at androidx.media3.common.FlagSet$Builder.tryStartForeground(SourceFile:1)
    at org.oxycblt.auxio.music.system.IndexerService.onIndexingStateChanged(SourceFile:145)
    at org.oxycblt.auxio.music.MusicRepositoryImpl.emitIndexingProgress(Unknown Source:94)
    at org.oxycblt.auxio.music.MusicRepositoryImpl$emitIndexingProgress$1.invokeSuspend(SourceFile:13)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(Unknown Source:8)
    at kotlinx.coroutines.DispatchedTask.run(Unknown Source:94)
    at androidx.fragment.app.Fragment$4.run(SourceFile:43)
    at kotlinx.coroutines.scheduling.TaskImpl.run(Unknown Source:2)
    at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(SourceFile:92)
OxygenCobalt commented 5 months ago

Ah, this error again @nitinsooni. Some OEMs don't allow me to start foreground services even when I'm allowed to. This is sometimes a timing issue, but I have the hunch that it's deliberate on behalf of some OEMs as to make third party apps less desirable compared to the baked-in OEM bloatware that coincidentally just...doesn't suffer from the same issues.

nitinsooni commented 5 months ago

That loading dialog on startup which groups everything up in-memory on startup, maybe caching that also would be good Idea because i didn't see any performance issue with other foss music players or proprietary ones like samsung music. They just opens and works fine.

And that error, you have any fix for that? Because it now just occurs like ever alternate day and its take lot of time to load around 5000 songs.

OxygenCobalt commented 5 months ago

That loading dialog on startup which groups everything up in-memory on startup, maybe caching that also would be good Idea because i didn't see any performance issue with other foss music players or proprietary ones like samsung music. They just opens and works fine.

Perhaps. I don't know whether a query over a big database will be more efficient, but perhaps grouping is far less efficient than I realized.

Not budging on in-memory loading though. Try using something like Phonograph and realize how SLOW and janky it is. That's the consequence of distributing the loading across the whole app.

And that error, you have any fix for that? Because it now just occurs like ever alternate day and its take lot of time to load around 5000 songs.

Nope. It's usually your phone. I assume it's a Samsung given that you brought up their app. They are notorious for killing foreground services for no reason and otherwise making things miserable for third-party apps. My theory is that this is intentional throttling to make you use Samsung Music (Which is also loaded up with ads like other awful Samsung bloatware). The most I can do is artificially introduce a delay in startup to appease their terrible ROM, but that means you need to wait another couple of seconds on top of loading.

nitinsooni commented 5 months ago

Nah that would just increase delay for others. I'm fine with tapping a retry button until some solid fix comes.

OxygenCobalt commented 5 months ago

There's no real fix @nitinsooni. This issue has existed for years (That's why my hunch is that it's intentional). Just stop getting samsung and get a pixel instead, which doesn't suffer from this.

Hund commented 2 months ago

I would love to see some message telling me about adding a duplicate song or not. I find it quite annoying to have the same tracks multiple times in my playlists.