aome510 / spotify-player

A Spotify player in the terminal with full feature parity
MIT License
3.36k stars 146 forks source link

Empty UI when playing a podcast episode #359

Open Schnouki opened 7 months ago

Schnouki commented 7 months ago

Is your feature request related to a problem? Please describe.

If I listen to a podcast on my phone, then switch to spotify_player on my laptop, then spotify_player doesn't show anything about the active episode. I can play it or pause it alright (never tried more), but there's nothing displayed about it. This is pretty confusing.

Describe the solution you'd like

Basic information about the current episode should be displayed.

Describe alternatives you've considered

None.

Additional context

There's actually no need for full support for podcasts here (although that could be nice). Just, you know, show what's currently playing :)

Screenshot Quick example: a podcast is playing (as shown in the Spotify Web UI), the current player is `spotify_player` on my laptop (device name "Magni"), but nothing is visible in the "Playback" section of the TUI: ![image](https://github.com/aome510/spotify-player/assets/64833/03f0a808-ff58-4eb0-ada1-01c1e4c008b2)
BinaryFly commented 5 months ago

I tried working on this but I'm afraid I don't have enough knowledge of rust or this project to add this change. But this is a nice pointer for someone that might want to continue working on this later.

[!NOTE] This is the first time ever for me to try my hand at a rust project, so if my code is wrong in some places and there is a better optimalisation for this, feel free to criticize it (so I can learn from it) and to suggest a different approach.

So first off this seemed like a nice first issue because my thought was that I should just change the formatted strings in the playback area to the right info for a podcast. This was relatively easy, although I did expect some polymorphism between FullEpisode types (the type for a podcast episode) and the FullTrack types, which there wasn't really.

I want to document my changes here for future reference (there's probably an easier way to do this using git, but I haven't really pushed anything so I figured I'd just show the diffs here)

First off I created a new struct in model.rs, this was not really necessary but it kept a clear overview for me as to what the changes were between episodes and tracks.

// a wrapper for tracks and episodes so we have a general talking point for the program to retrieve
// info from these two PlayableItems
pub struct PlayableItemWrapper {
    pub item: rspotify_model::PlayableItem,
}

impl PlayableItemWrapper {
    pub fn new(playable_item: &rspotify_model::PlayableItem) -> Self {
        PlayableItemWrapper {
            item: playable_item.clone()
        }
    }

    pub fn images(&self) -> &Vec<rspotify::model::Image> {
        match &self.item {
            rspotify::model::PlayableItem::Track(track) => &track.album.images,
            rspotify::model::PlayableItem::Episode(show) =>&show.images
        }
    }

    pub fn duration(&self) -> chrono::Duration {
        match &self.item {
            rspotify_model::PlayableItem::Track(track) => track.duration,
            rspotify_model::PlayableItem::Episode(episode) => episode.duration,
        }
    }

    // formats the track name for the owned PlayableItem
    pub fn trackformat(&self) -> String {
        match &self.item {
            rspotify_model::PlayableItem::Track(track) => { 
                if track.explicit {
                    format!("{} (E)", track.name)
                } else {
                    track.name.clone()
                }
            },
            rspotify_model::PlayableItem::Episode(episode) => {
                if episode.explicit {
                    format!("{} (E)", episode.name)
                } else {
                    episode.name.clone()
                }
            },
        }
    }

    // formats the artists or podcast hosts for the owned PlayableItem
    pub fn artistsformat(&self) -> String {
        match &self.item {
            rspotify_model::PlayableItem::Track(track) => {
                crate::utils::map_join(&track.artists, |a| &a.name, ", ")
            },
            rspotify_model::PlayableItem::Episode(episode) => {
                episode.show.publisher.clone()
            }

        }
    }

    // formats the album or show name for the owned PlayableItem
    pub fn albumformat(&self) -> String {
        match &self.item {
            rspotify_model::PlayableItem::Track(track) => track.album.name.clone(),
            rspotify_model::PlayableItem::Episode(episode) => episode.show.name.clone()
        }
    }
}

I also implemented a new function to get the right image url for any vector of images in utils.rs:

pub fn get_image_url(images: &Vec<rspotify::model::Image>) -> Option<&str> {
        if images.is_empty() {
            None
        } else {
            Some(&images[0].url)
        }
 }

Then I changed some things for the function render_playback_window and the private functions it called in the same file, here I use most of the previously seen methods implemented via the new struct PlayableItemWrapper:

@@ -15,7 +15,8 @@ pub fn render_playback_window(

     let player = state.player.read();
     if let Some(ref playback) = player.playback {
-        if let Some(rspotify::model::PlayableItem::Track(ref track)) = playback.item {
+        if let Some(ref item) = playback.item {
+            let item_wrapper = PlayableItemWrapper::new(item);
             let (metadata_rect, progress_bar_rect) = {
                 // allocate the progress bar rect
                 let (rect, progress_bar_rect) = {
@@ -48,7 +49,7 @@ pub fn render_playback_window(
                             (hor_chunks[1], ver_chunks[0])
                         };

-                        let url = crate::utils::get_track_album_image_url(track).map(String::from);
+                        let url = crate::utils::get_image_url(item_wrapper.images());
                         if let Some(url) = url {
                             let needs_clear = if ui.last_cover_image_render_info.url != url
                                 || ui.last_cover_image_render_info.render_area != cover_img_rect
@@ -98,16 +99,16 @@ pub fn render_playback_window(
             };

             if let Some(ref playback) = player.buffered_playback {
-                render_playback_text(frame, state, ui, metadata_rect, track, playback);
+                render_playback_text(frame, state, ui, metadata_rect, item, playback);
             }

             let progress = std::cmp::min(
                 player
                     .playback_progress()
                     .context("playback should exist")?,
-                track.duration,
+                item_wrapper.duration()
             );
-            render_playback_progress_bar(frame, state, ui, progress, track, progress_bar_rect);
+            render_playback_progress_bar(frame, state, ui, progress, item, progress_bar_rect);
         } else {
             tracing::warn!("Got a non-track playable item: {:?}", playback.item);
         }
@@ -143,17 +144,19 @@ pub fn render_playback_window(
     Ok(())
 }

+
 fn render_playback_text(
     frame: &mut Frame,
     state: &SharedState,
     ui: &UIStateGuard,
     rect: Rect,
-    track: &rspotify_model::FullTrack,
+    track: &rspotify_model::PlayableItem,
     playback: &SimplifiedPlayback,
 ) {
     // Construct a "styled" text (`playback_text`) from playback's data
     // based on a user-configurable format string (app_config.playback_format)
     let format_str = &state.configs.app_config.playback_format;
+    let item_formatter = PlayableItemWrapper::new(track);

     let mut playback_text = Text::default();
     let mut spans = vec![];
@@ -192,19 +195,15 @@ fn render_playback_text(
                     } else {
                         &state.configs.app_config.play_icon
                     },
-                    if track.explicit {
-                        format!("{} (E)", track.name)
-                    } else {
-                        track.name.clone()
-                    }
+                    item_formatter.trackformat()
                 ),
                 ui.theme.playback_track(),
             ),
             "{artists}" => (
-                crate::utils::map_join(&track.artists, |a| &a.name, ", "),
+                item_formatter.artistsformat(),
                 ui.theme.playback_artists(),
             ),
-            "{album}" => (track.album.name.to_owned(), ui.theme.playback_album()),
+            "{album}" => (item_formatter.albumformat(), ui.theme.playback_album()),
             "{metadata}" => (
                 format!(
                     "repeat: {} | shuffle: {} | volume: {} | device: {}",
@@ -237,13 +236,14 @@ fn render_playback_progress_bar(
     state: &SharedState,
     ui: &mut UIStateGuard,
     progress: chrono::Duration,
-    track: &rspotify_model::FullTrack,
+    track: &rspotify_model::PlayableItem,
     rect: Rect,
 ) {
+    let track_wrapper = PlayableItemWrapper::new(track);
     // Negative numbers can sometimes appear from progress.num_seconds() so this stops
     // them coming through into the ratios
     let ratio =
-        (progress.num_seconds() as f64 / track.duration.num_seconds() as f64).clamp(0.0, 1.0);
+        (progress.num_seconds() as f64 / track_wrapper.duration().num_seconds() as f64).clamp(0.0, 1.0);

     match state.configs.app_config.progress_bar_type {
         config::ProgressBarType::Line => frame.render_widget(
@@ -254,7 +254,7 @@ fn render_playback_progress_bar(
                     format!(
                         "{}/{}",
                         crate::utils::format_duration(&progress),
-                        crate::utils::format_duration(&track.duration),
+                        crate::utils::format_duration(&track_wrapper.duration()),
                     ),
                     Style::default().add_modifier(Modifier::BOLD),
                 )),
@@ -268,7 +268,7 @@ fn render_playback_progress_bar(
                     format!(
                         "{}/{}",
                         crate::utils::format_duration(&progress),
-                        crate::utils::format_duration(&track.duration),
+                        crate::utils::format_duration(&track_wrapper.duration()),
                     ),
                     Style::default().add_modifier(Modifier::BOLD),
                 )),

Now after these changes I figured it would work and the Playback UI section would no longer be empty if I reran cargo. Unfortunately it didn't work. I looked in the logs and saw the following (I added the upper warning with every event change for more clarity):

2024-04-01T14:24:16.676929Z  WARN spotify_player::streaming: Got event Changed { old_track_id: SpotifyId { id: 331090924749660149588610714563232636154, audio_type: Track }, new_track_id: SpotifyId { id: 295904964958321251011372098311617213191, audio_type: Podcast } }
2024-04-01T14:24:16.677344Z  WARN spotify_player::streaming: Failed to convert a `librespot` player event into `spotify_player` player event: InvalidType

It seems the program doesn't recognize the PlayerEvent, why this is is easily seen by taking a look at streaming.rs, we only support the type TrackId whenever an event changes, in the case of a podcast the id is of the type EpisodeId or ShowId (sorry I didn't check). I didn't dare to change this because I don't really know how to do this cleanly while preserving the code structure, therefore I think this is better left to someone who has more experience with the project or rust. If there is an easy fix for this (i.e. casting an EpisodeId into a TrackId somewhere if that's possible in rust) then by all means suggest it and I'll see if I can still implement it.