search

elementary / music

Music player and library designed for elementary OS
https://elementary.io
GNU General Public License v3.0
145 stars 49 forks source link

Skip invalid files instead of stopping playback. #711

Closed vjr closed 2 years ago

vjr commented 2 years ago

If you enqueue invalid files (such as I did when selecting two audio and one text file) then playback stops at the invalid file.

This PR simply skips to the next (or previous, if the previous button pressed) file instead of stopping at the invalid file.

Example screenshot:

image

vjr commented 2 years ago

Also tested with multiple invalid files adjacent to each other and it skips over all of them to the next (or previous) valid audio file.

Marukesu commented 2 years ago

i believe filtering the filechooser/drag'n'drop/Application.open() code to music files only an better solution. that will also stop the document portal of exporting unneeded/unused files.

vjr commented 2 years ago

i believe filtering the filechooser/drag'n'drop/Application.open() code to music files only an better solution. that will also stop the document portal of exporting unneeded/unused files.

Yes, that would be better. I'm hoping this PR can be a stop-gap merged until the proper excluding of invalid files is done?

treppenwitz03 commented 2 years ago

In https://github.com/elementary/music/blob/main/src/MainWindow.vala#L119 maybe we can put a function like GLib.ContentType.guess to get the mime type of the file dropped, and only append the file to the file array if it has audio mime type

Marukesu commented 2 years ago

I'm hoping this PR can be a stop-gap merged until the proper excluding of invalid files is done?

yeah, this PR is still valid even if the filtering is done, since there's can be errors about missing codecs or if the file was moved/deleted. one thing that i was think is, if it make sense to show an notification/toast when an error occurs.

vjr commented 2 years ago

I'm completely useless when it comes to UI aspects haha so I welcome commits to this branch if we want the toast in this PR itself.

In the meantime I'll look into the suggestion about checking mime type...

treppenwitz03 commented 2 years ago

@vjr maybe we can call a signal in the queue_files function in PlaybackManager if the mimetype of the dropped or 'opened by terminal' files are not audio mime types. Then connect the toast to it, then if the codecs of the file is missing, we skip those invalid files with this PR's fix and call another toast to say that probably the codec for the audio is missing?

vjr commented 2 years ago

@ZenitsuDev I took the liberty to rename the signal from process_error to invalids_found because the former feels like a method name and the latter more like a signal name, hope it's okay?

Let me know if you have any further review comments or suggest further changes/additions and if not then we can change this PR from draft to "ready for review" :-)

treppenwitz03 commented 2 years ago

@vjr I am satisfied and I think we can mark it as ready for review😁