Differential D8989

show passive notification when a track fail to play
ClosedPublic
Actions

Authored by mgallien on Nov 24 2017, 10:21 PM.

Details

Reviewers

astippich

Group Reviewers

Elisa

Commits

R255:2049976bc77d: show passive notification when a track fail to play

Summary

currently shows a passive notification when a track fail to play

the name of the track is currently an url that is not easy to understand

Test Plan

If I have a track where I force it to be in error (truncate to 0 size), the notification shows

Diff Detail

Repository

R255 Elisa

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

mgallien created this revision.Nov 24 2017, 10:21 PM

I tried the patch by deleting tracks that are currently in the playlist. Notification works good.
I found another bug during testing, but I think it's unrelated to this revision. I added an album to the playlist and started playback. Then deleted the whole album folder. When you want to start playback of another track it throws the error. Then it tries the next track which also throws an error. The issue is that it does not stop at the end of the playlist, but instead but restarts at the first entry, and then repeats forever

src/PassiveNotification.qml
21	why do you import it this way? also works without "as QQC2" here and shouldn't be needed, since no controls 1 are imported, or am I wrong?

mgallien marked an inline comment as done.Nov 27 2017, 9:59 PM

mgallien added inline comments.

src/PassiveNotification.qml
21	This file is imported from Kirigami2 with some modifications when needed.

In D8989#171903, @astippich wrote:

I tried the patch by deleting tracks that are currently in the playlist. Notification works good.
I found another bug during testing, but I think it's unrelated to this revision. I added an album to the playlist and started playback. Then deleted the whole album folder. When you want to start playback of another track it throws the error. Then it tries the next track which also throws an error. The issue is that it does not stop at the end of the playlist, but instead but restarts at the first entry, and then repeats forever

I am trying to identify and fix the reason why tracks are not put invalid in the playlist when they are removed from database. This will involve adding more automatic tests. Do you also want to review the c++ only modifications I do ?

In D8989#172717, @mgallien wrote:

In D8989#171903, @astippich wrote:

I tried the patch by deleting tracks that are currently in the playlist. Notification works good.
I found another bug during testing, but I think it's unrelated to this revision. I added an album to the playlist and started playback. Then deleted the whole album folder. When you want to start playback of another track it throws the error. Then it tries the next track which also throws an error. The issue is that it does not stop at the end of the playlist, but instead but restarts at the first entry, and then repeats forever

I am trying to identify and fix the reason why tracks are not put invalid in the playlist when they are removed from database. This will involve adding more automatic tests. Do you also want to review the c++ only modifications I do ?

That is probably unnecessary. You certainly can code better than me

when a track is in error, show the file name or the url if it is not a local file

track with error becomes invalid in playlist and improve way to notify the user

mgallien retitled this revision from [WIP]: show passive notification when a track fail to play to show passive notification when a track fail to play.Dec 6 2017, 8:08 PM

everything is working fine

This revision is now accepted and ready to land.Dec 6 2017, 8:47 PM

Closed by commit R255:2049976bc77d: show passive notification when a track fail to play (authored by mgallien). · Explain WhyDec 6 2017, 8:59 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/MediaServer.qml (14 lines)
M			src/PassiveNotification.qml (52 lines)
M			src/manageaudioplayer.h (2 lines)
M			src/manageaudioplayer.cpp (10 lines)
M			src/mediaplaylist.h (3 lines)
M			src/mediaplaylist.cpp (17 lines)
M			src/musiclistenersmanager.h (2 lines)
M			src/musiclistenersmanager.cpp (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	22906	6804d87		Nov 24 2017, 10:21 PM	★	★
Diff 2	23542	4a93b8a	when a track is in error, show the file name or the url if it is not a local…	Dec 6 2017, 6:23 AM	★	★
Diff 3	23584	4a93b8a	track with error becomes invalid in playlist and improve way to notify the user	Dec 6 2017, 8:08 PM	★	★
Diff 4	23587	25448f4	R255:2049976bc77dc40828a78cf935cb299cef14de6c	Dec 6 2017, 8:59 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
b7c686c48580	80bda8d86df6	14e7e3f42b43	Matthieu Gallien	track with error becomes invalid in playlist and improve way to notify the user	Dec 6 2017, 8:05 PM
14e7e3f42b43	2cb1b9e20636	4a93b8a27296	Matthieu Gallien	show passive notification when a track fail to play (Show More…)	Nov 24 2017, 10:19 PM