Bug fixes & minor improvements

[Hanouta]

This contains a couple of bug fixes and also minor improvements, such as:

• crash fixes
• restoring the last played Episode after vocal has been closed
• using CDATA when exporting the Podcasts, to avoid issues with some special characters
• add a check if the user has configured gpodder, otherwise don't try to send the updated data to gpodder
• only add Episodes(and Feeds) which contain a media file

Fixes: #444
Fixes: #436
Fixes: #404
Fixes: #384
Fixes: #307
Fixes: #211

[Hanouta]

@nathandyer
Could you try how well the branch from this PR works for you? Because on my main OS(Arch based) it works pretty well(there are still a few bugs that I haven't fixed yet, but nothing major), but when I try it in a Elementary OS Virtual Machine the program sometimes freezes and causes a high CPU usage. I've experienced that during the migration process (after upgrading from 2.4.2) and sometimes also right after startup. Basically I would like to know if that's just an issue with my VM or if it's a Elementary/Ubuntu specific problem.

EDIT: Just installed Elementary on a different drive, it seems to work just fine. Not sure why I had this issue in the VM.
EDIT2: It was because I didn't use the VMSVGA graphics adapter in VirtualBox.

[nathandyer]

@brn9hrd7 First and foremost, thank you so much for all the work you've been pouring into Vocal over the past few weeks. It's a tremendous help to the project and I can't sincerely thank you enough.

I tried this branch out last week and started to go through it for a code review, but there was so much going on in it I ran out of time to do it properly. I'm going to dig back in tonight and try to give it the look it deserves.

That being said, in the future, please be sure to limit each pull request to a specific change or fix, because otherwise it becomes almost impossible to review the code and address every single component inside it. For example, just on what I saw last week, some of the things worked, but for example one of your commits said it restored the last playing media, but I was unable to confirm that (it didn't work on my side). If I were to approve the PR then that bug would automatically get marked as fixed, although it's not. As a good practice (not just in Vocal but in all open source projects), having a single PR per fix will lead to the fixes landing faster and it's easier for both you and the maintainer.

I'll dig in later tonight, so be prepared for notifications over the next couple days. :)

[nathandyer]

@brn9hrd7 Code-wise everything looked really good! I appreciate you adhering to the code style, a lot of contributors have trouble sticking to it, so good on you for that! All the changes you made were understandable, and in some cases cleaned up some really old code that had been hanging on for far too long. Thanks for the attention to detail!

I tested out all your changes and can confirm that almost all of them work for me, however it still does not restore the last-playing media no matter what I seem to do, and also I never saw the iTunes Top 100 loading label. Are both working on your machine?

[Hanouta]

Yes, restoring the episode and the loading label are both working here.
Regarding the episode restoring, did you by any chance close the window & relaunch the Application before it finished updating the podcasts?

Not sure why you didn't see the loading label, maybe your internet connection is just too fast or something(it starts loading the Top 100 immediately at startup after all). You could try deleting the ~/.cache/vocal directory, so it will take a bit longer :D.

Question regarding the desired behavior of the position restoring:
Should it restore the Episode position every time or only after launching the program? E.g. when playing Episode X, then start Episode Y before X finished, then play X again... Should it start at 0 or restore the position? Atm it will only restore the position from the last played episode after launching the program.

[nathandyer]

@brn9hrd7 To your first point, let me try it on a different computer on a slower connection to see if either makes a difference about it restoring positions or showing the progress bar. I should have time this evening to that, and I'll let you know. And yes, I was waiting until the podcasts finished updating before I closed.

On your second point, Vocal should really start playing episodes back at whatever point they were last left off, whether the user jumped to another episode or quit the app, or whatever happened. That becomes particularly important with the addition of the gpodder.net sync in our upcoming release (so for example, if someone listens to episode X on Vocal, they should be able to pick up at the exact place they left off of episode X from Vocal on their mobile device, then when Vocal syncs again it should see how far they made it on their mobile device and then pick back up at that new position).

[Hanouta]

showing the progress bar.

There isn't a progress "bar", it just shows the progress by changing the label text.

On your second point, Vocal should really start playing episodes back at whatever point they were last left off, whether the user jumped to another episode or quit the app, or whatever happened. That becomes particularly important with the addition of the gpodder.net sync in our upcoming release (so for example, if someone listens to episode X on Vocal, they should be able to pick up at the exact place they left off of episode X from Vocal on their mobile device, then when Vocal syncs again it should see how far they made it on their mobile device and then pick back up at that new position).

Gotcha, will change that.

[Hanouta]

Just added another commit, it should restore the position every time now. Note that it can take up to half a second(after it starts playing) for the position to get restored, because I couldn't find a way to do it immediately (there is a ready() signal in ClutterGst.Player, but that didn't seem to do anything).

[nathandyer]

@brn9hrd7 I just tested your branch out on a laptop with a clean install that has never had Vocal running on it at any point. I still never saw the changing of the label text, unless I'm just missing something. Please see the screenshot I grabbed:

As for restoring position, I'm afraid I could not replicate it working. It never worked for me. Specifically:

• I tried switching episodes, pausing an episode, and exiting the app to trigger it to save the position.
• When opening the app after an episode had already been playing, clicking the Play button should pick back up with the previous episode at the last spot. Also, the episode should be visible in the playback box.
• If I manually open the podcast and click the episode in the episode list, if the episode is not downloaded I do not see any indication that it tries to resume playback. It just starts playing the episode at the beginning.
• If I do the same but with a downloaded episode, I can see it try to seek after about half a second (it fills the progress bar and sets the time remaining label to an invalid number), but then fails to seek and starts back at the beginning.

I attempted it again on my primary development machine and confirmed the same behaviour.

[Hanouta]

Is it possible that you were just using the wrong branch(e.g. master)? As you can see here (DirectoryView Line 106) the label text has been changed, but on your screenshot it still shows the old text.

I just made a screen recording on a fresh installation of Pop!_OS 20.04, to show the loading label and also the episode & position restoring: https://streamable.com/ot10i3
Note that I ran across one bug/scenario where it doesn't save the position, which is the reason why it doesn't restore the position in the video at the 3 minute mark. Will have a patch for that soon.

[cognition9144]

Hi @nathandyer , may I ask about the condition of this PR? I'm looking forward to it because it solves the CDATA issue.

[nathandyer]

@brn9hrd7 Hi! First, I'm so sorry this has taken so long to get back to you. I was away for medical reasons. I'd like to go ahead and merge this in. If you're still available and willing to do so, would you mind reviewing the remaining conflicts? Once those are sorted out, I'll merge this in immediately.

Again, thank you so much, this is absolutely outstanding work.

[Hanouta]

Conflicts should be resolved now.