5
\$\begingroup\$

I'm working on an audio player with a terminal interface. I'm designing it for eventual use on a Raspberry Pi with a 480x320px display.

I've broken the code into three main files with the intention of keeping the business logic (player.py) and graphics (view.py) separated and connected only through the main file. I've also included a configuration file where I'm storing future user options and text strings.

I have more functionality planned, so I'd like to make sure I have a solid foundation now while the complexity is low.

Any comments on the architecture, style, or efficacy of the code are welcome. The code is also available on on github.

main.py

import sys, os
from pynput import keyboard
from functools import partial
import cfg
from view import View
from player import Player
exit_signal: bool = False
def on_press(key: keyboard.KeyCode, view: View, player: Player):
 """Handle input"""
 global exit_signal
 key = str(key).strip('\'')
 if str(key) == 'p':
 view.notify('Playing...')
 player.play()
 elif key == 'a':
 view.notify('Paused')
 player.pause()
 elif key == 'n':
 view.notify('Skipping Forward...')
 player.skip_forward()
 elif key == 'l':
 view.notify('Skipping Back...')
 player.skip_back()
 elif key == 'q':
 view.notify('Exiting...')
 exit_signal = True
 del player
 del view
 return
 view.update_ui(player.get_metadata())
def tick(view: View, player:Player):
 """For functions run periodically"""
 metadata = player.get_metadata()
 view.update_ui(metadata)
view = View()
player = Player()
view.notify("Ready!")
with keyboard.Listener(on_press=partial(on_press, view=view, player=player)) as listener:
 while exit_signal == False:
 tick(view, player)
 listener.join() # merge to one thread
 os.system('reset') # clean up the console

player.py

import os
import vlc # swiss army knife of media players
import cfg
from mutagen.easyid3 import EasyID3 as ID3 # audio file metadata
music_path = os.path.abspath(os.path.join(__file__, '../../music'))
playlist_path = os.path.abspath(os.path.join(__file__, '../../playlists'))
class Player:
 """Track player state, wrap calls to the VLC plugin, and handle scanning for media."""
 def __init__(self):
 self.song_list = []
 self.song_idx = 0
 self.song_idx_max = 0
 self.vlc = vlc.Instance()
 self._scan_library()
 self.player = self.vlc.media_player_new(self.song_list[self.song_idx])
 self._update_song()
 def __del__(self):
 del self.vlc
 return
 def _scan_library(self):
 for dirpath, _, files in os.walk(music_path):
 for file in files:
 filepath = os.path.join(dirpath, file)
 _, ext = os.path.splitext(filepath)
 if ext in cfg.file_types:
 self.song_list.append(filepath)
 self.song_idx_max += 1
 def _get_id3(self, key: str):
 metadata = ID3(self.song_list[self.song_idx])
 return str(metadata[key]).strip('[\']')
 def _update_song(self):
 if self.song_list and self.song_idx < len(self.song_list):
 self.media = self.vlc.media_new(self.song_list[self.song_idx])
 self.media.get_mrl()
 self.player.set_media(self.media)
 def get_metadata(self):
 """Return a dictionary of title, artist, current runtime and total runtime."""
 if (not self.song_list) or (self.song_idx < 0):
 return None
 else:
 # default states when not playing a track are negative integers
 curr_time = self.player.get_time() / 1000 # time returned in ms
 if (curr_time < 0):
 curr_time = 0
 run_time = self.player.get_length() / 1000
 if (run_time < 0):
 run_time = 0
 playing = False
 else:
 playing = True
 info = {"playing": playing,
 "title": self._get_id3('title'),
 "artist": self._get_id3('artist'),
 "curr_time": curr_time,
 "run_time": run_time}
 return info
 def play(self):
 """Start playing the current track."""
 self.player.play()
 def pause(self):
 """Pause the current track. Position is preserved."""
 self.player.pause()
 def skip_forward(self):
 """Skip the the beginning of the next track and start playing."""
 if (self.song_idx < self.song_idx_max):
 self.song_idx += 1
 self._update_song()
 self.play()
 def skip_back(self):
 """Skip to the beginning of the last track and start playing."""
 if (self.song_idx > 0):
 self.song_idx -= 1
 self._update_song()
 self.play()

view.py

import cfg # settings
import curses # textual user interface
from datetime import timedelta
class View:
 """Wrap the python Curses library and handle all aspects of the TUI."""
 def __init__(self):
 self.screen = curses.initscr()
 self.max_y_chars, self.max_x_chars = self.screen.getmaxyx()
 self._draw_border()
 self.line1 = self.max_y_chars - 4
 self.line2 = self.max_y_chars - 3
 self.line3 = self.max_y_chars - 2
 self.screen.refresh()
 def __del__(self):
 """Restore the previous state of the terminal"""
 curses.endwin()
 def _draw_border(self):
 self.screen.border(0)
 self.screen.addstr(0, (self.max_x_chars - len(cfg.title)) // 2, cfg.title)
 def _set_cursor(self):
 self.screen.move(2, len(cfg.prompt_en) + 2)
 def _clear_line(self, line: int):
 self.screen.move(line, 1)
 self.screen.clrtoeol()
 self._draw_border()
 def _strfdelta(self, tdelta: timedelta):
 """Format a timedelta into a string"""
 days = tdelta.days
 hours, rem = divmod(tdelta.seconds, 3600)
 minutes, seconds = divmod(rem, 60)
 time_str = ''
 if days > 0:
 time_str += str(days) + ' days, '
 if hours > 0:
 time_str += str(hours) + ' hours '
 time_str += str(minutes)
 time_str += ':'
 if (seconds < 10):
 time_str += '0'
 time_str += str(seconds)
 return time_str
 def _update_progress_info(self, metadata):
 if metadata is None:
 return
 run_time = metadata["run_time"]
 curr_time = metadata["curr_time"]
 if run_time == 0:
 return
 percent = int((curr_time / run_time) * 100)
 run_time_str = self._strfdelta(timedelta(seconds=run_time))
 curr_time_str = self._strfdelta(timedelta(seconds=curr_time))
 time_str = curr_time_str + cfg.time_sep_en + run_time_str + ' (' + str(percent) + '%)'
 # two border characters
 progress_bar_chars = self.max_x_chars - 2
 fill_count = int(progress_bar_chars * curr_time / run_time)
 progress_fill = cfg.prog_fill * fill_count
 progress_void = ' ' * (progress_bar_chars - fill_count)
 progress_bar = progress_fill + progress_void
 self.screen.addstr(self.line2, 1, time_str)
 self.screen.addstr(self.line3, 1, progress_bar)
 def notify(self, string: str):
 """Add a string to the top of the window."""
 self._clear_line(1)
 self.screen.addstr(1, 1, string)
 self._set_cursor()
 self.screen.refresh()
 def update_ui(self, metadata: dict):
 """Update track metadata and progress indicators."""
 self._clear_line(self.line1)
 self._clear_line(self.line2)
 self._clear_line(self.line3)
 if metadata is None:
 return
 else:
 if metadata['playing']:
 info_line = metadata['title'] + cfg.song_sep_en + metadata['artist']
 self.screen.addstr(self.line1, 1, info_line)
 self.screen.addstr(2, 1, cfg.prompt_en)
 self._update_progress_info(metadata)
 self._draw_border()
 self._set_cursor()
 self.screen.refresh()

cfg.py

# text strings
title = 'OpenDAP v0.0.0.1'
no_media_en = "Nothing is playing!"
no_load_en = "..."
no_progress_en = "Cannot display progress."
prompt_en = "[P]lay, P[a]use, [N]ext, [L]ast, [Q]uit ▶ "
song_sep_en = " by "
time_sep_en = " of "
# file types to scan for
file_types = {'.mp3', '.flac'}
# progress bar filler
prog_fill = '▒'

Thank you for your time.

asked Jan 21, 2020 at 20:58
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Your code is pretty good.

  • You have some Pythonic style problems like on_press should have two blank lines before it, mypy probably would complain about exit_singal: bool = False and un-Pythonic if (...): statements. And so I recommend that you run a linter or two on your code.

  • Personally I don't like view.notify('Playing...') and then player.play(). Personally I would prefer if view and player had the same interface for signals. This would mean that you would have:

    if key == 'p':
     view.play()
     player.play()
    
  • Personally I would prefer raise SystemExit(0) rather than exit_signal. This reduces the size of the global scope. You should be able to achieve the same results with a try finally.

  • If you utilize SystemExit and make view and player have the same interface. Then you could convert on_press to be super simple, by using a dictionary:

    INTERFACE = {
     'p': 'play',
     'a': 'pause',
     'n': 'skip_forward',
     'l': 'skip_back',
     'q': 'quit',
    }
    def on_press(key: keyboard.KeyCode, view: View, player: Player):
     key = str(key).strip("'")
     function = INTERFACE.get(key, None)
     if function is not None:
     getattr(view, function)()
     getattr(player, function)()
     view.update_ui(player.get_metadata())
    
  • I've found it to be very rare to need to use del, or ever define __del__. Maybe you really need it, maybe you don't?

  • You can use f'{minutes}:{seconds:0>2}' so that the seconds are padded in View._strfdelta.
answered Jan 22, 2020 at 9:16
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, there's a lot of great improvements in this answer. Marking it as the solution for now. \$\endgroup\$ Commented Jan 23, 2020 at 18:37

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.