lucasasselli / garmin-podcasts

Garmin Podcasts is a Garmin Connect IQ podcast app powered by Podcast Index. No external service or subscription required: all you need is you watch!
GNU General Public License v3.0
76 stars 17 forks source link

playback order #18

Closed lucasnz closed 2 years ago

lucasnz commented 2 years ago

Hi, I'm having trouble with the playback order (I'm currently only using one podcast). The order displays correctly (newest podcast first) on the download episodes screen, but seems to be out of order on the playback queue screen (and resulting playback). I really need the podcasts to play oldest to newest (otherwise they don't always make sense). Any idea what's causing them to be out of order? How is the order of the playback queue set?

Perhaps we could add an option to sort the playback queue (e.g. oldest to newest)?

lucasasselli commented 2 years ago

Hi lucasnz, the playback is set in the order of the episode selection.

lucasnz commented 2 years ago

Thanks, that makes sense, although is difficult when the episodes don't have a number or date in the title. Is there no way to easily add an option to sort the playback list by date?

lucasasselli commented 2 years ago

Hi @lucasnz, episodes should be sorted by date in the queue view in version 3.3.0.

lucasnz commented 2 years ago

Thanks @lucasasselli, I have updated and it has changed the order. But it's not displaying in the order I'm expect. Is it using pubDate from the rss feed? For the particular rss feed it has the field formatted like this:

        <item>
            ...
            <pubDate>Tue, 08 Mar 2022 05:01:00 +0000</pubDate>
            ...
        </item>
lucasasselli commented 2 years ago

I am using the published_parsed value extracted by feedparser. More info here. It might be an unsupported format?

lucasnz commented 2 years ago

It appears to be parsing the date correctly. If I navigate to: https://us-central1-garminpodcasts-307017.cloudfunctions.net/feed-parser-2?feedUrl=&max=300

Then I get valid json returned. Episodes are sorted newest to oldest and display a valid unix timestamp.

I wonder if the issue lies in Utils.mc Is the function sortArrayField correct? Should line 34 be: Utils.arraySwap(array, i, i+1); Is the function arraySwap correct? Should line 44 be: array[j] = temp;

lucasasselli commented 2 years ago

😢 oh man! You are absolutely right!!!! Just published a patch it should be on the store in the next few hours.

lucasnz commented 2 years ago

@lucasasselli - that seems to have broken something. Now I can't get into the Playback queue screen at all. Perhaps the sort algorithm is stuck in an infinite loop??

lucasnz commented 2 years ago

With those two functions, do they need to return the array? Or does that variable have a "universal" scope? It looks to me as if an infinite loop occurs, because the swap never actually gets feed back to the sort function. Then does the sort function need to return the sorted array??

lucasnz commented 2 years ago

That was enough to get me to download the SDK and test it out. This PR seems to fix it: https://github.com/lucasasselli/garmin-podcasts/pull/21

lucasasselli commented 2 years ago

My understanding was that the array was passed by reference: I didn't observe any issue so far, but likely I didn't do enough testing.

lucasasselli commented 2 years ago

Integrated in 3.3.4!

lucasnz commented 2 years ago

Sort is working correctly now. I do wonder if it should be sorted oldest to newest - it currently defaults to newest to oldest. Or should this be a configurable setting. Options for playback queue sort could be: descending, ascending. and off.

The other feature, I think that would be helpful is to have an option to select all in the playback queue - such that the queue plays in the order displayed.

If you think it useful, I might look at adding these features.

lucasasselli commented 2 years ago

That's a good observation. I think that the order should be exclusively earliest episode first, and the playback should should follow that as well, not the order of selection.

lucasnz commented 2 years ago

I agree and have been working on some modifications to this end. I will submit a PR in a few days (hopefully - you know life can get busy)...

lucasnz commented 2 years ago

This seems to be working pretty well: https://github.com/lucasasselli/garmin-podcasts/pull/23

lucasasselli commented 2 years ago

I integrated the PR and reworked the code a little to remove the queue array and instead sort the episodes at playback time.

lucasnz commented 2 years ago

Nice. I just spun up in the emulator and it looks good. One comment, if settingQueueAutoSelect is true and the user selects not to start playback, I think it should enter the Queue menu. It seems "busy" doesn't get unset.

lucasnz commented 2 years ago

The upgrade reset my local subscriptions and downloads. Which meant I discovered a minor bug when you create a new subscription, then head straight to the download screen and it won't enter that section. Exiting the app and navigating back to downloads 'resolves' the issue...

lucasasselli commented 2 years ago

Good catch for the bug! Just pushed a fix for it! For the select all, maybe a good idea could be having a button in the queue instead of the settings. Something like "Select all" at the very top.

lucasnz commented 2 years ago

I like the way you have it now, so it plays all the media if settingsQueueAutoSelect is true, but suggest this tweak:

@@ -59,11 +59,11 @@ class MainMenu extends Ui.CompactMenu {
         var downloadedCount = getDownloadedSize();

         if (downloadedCount > 0) {

             // Episodes downloaded

             var autoQueue = Application.getApp().getProperty("settingQueueAutoSelect");

             if(autoQueue){

-                var prompt = new Ui.CompactPrompt(Rez.Strings.confirmPlayback, method(:startPlayback), null);

+                var prompt = new Ui.CompactPrompt(Rez.Strings.confirmPlayback, method(:startPlayback), WatchUi.pushView(new Queue(), new QueueDelegate(), WatchUi.SLIDE_LEFT));

                 prompt.show();

             }else{

                 WatchUi.pushView(new Queue(), new QueueDelegate(), WatchUi.SLIDE_LEFT);

             }

         } else {

The other option, could be to have a "play all episodes" menuitem at the top of the main menu - this could simply play all downloaded episodes (perhaps the sub-title could show the downloaded count). Then the settingsQueueAutoSelect property would become redundant and the playback queue could go back to how it was.

lucasasselli commented 2 years ago

I don't get the point of having the auto-select option + the queue at the same time! Is the idea to automatically add new episodes or to have a simple way to add all episodes at the same time?

lucasnz commented 2 years ago

I like the way it is at the moment, makes it super fast to play everything you've downloaded. I just thought one might want to remove some items from the queue (even when the auto select option is one) - but you could just skip those items... So, in short, I'm happy with it as is. We can probably close this now :)