Code review comment for lp:~samuel-buffet/entertainer/bug_310413

Revision history for this message
Samuel Buffet (samuel-buffet) wrote :

We now do care of the status of the answer we receive from the CDDB query.

I've also fixed a unicode/charset issue. At least for western occidental accentuated languages.

Samuel,

NOTE:

To test you'll need to find the 2 types of CD.

the one with on dictionnary result (status 200)
the one which gives a list of 3 dictionnaries (status 210 or 211)

(I had difficulties to find a type 200)

the diff:
=== modified file 'entertainerlib/frontend/medialibrary/music.py'
--- entertainerlib/frontend/medialibrary/music.py 2008-08-05 07:42:50 +0000
+++ entertainerlib/frontend/medialibrary/music.py 2009-01-08 21:46:01 +0000
@@ -601,65 +601,49 @@

         (query_status, query_info) = CDDB.query(disc_id)

- # FIXME: query_info contains code that we could use instead of TRY
- # EXCEPT
- #See CDDB documentation for more information.
+ #See CDDB documentation for more information
         #http://cddb-py.sourceforge.net/CDDB/README
-
- if query_info is not None:
- # query_info variable's type depends on how many matches we get.
- # If we get just one match then it's a map and if we get more
- # than one it's a list.
- try:
- title = query_info['title']
- self.artist = title[:title.index(' / ')]
- self.album = title[title.index(' / ') + 3:]
-
- # Get track titles
- (read_status, read_info) = CDDB.read(query_info['category'],
- query_info['disc_id'])
- cumulative_length = 0
- for i in range(disc_id[1]):
- if i + 4 == len(disc_id):
- # We must calculate last track length different way
- length = disc_id[len(disc_id) - 1] - cumulative_length
- else:
- # Calculate track length in seconds
- length = (disc_id[i+3] - disc_id[i+2]) / 75
- cumulative_length = cumulative_length + length
-
- track_title = read_info['TTITLE' + str(i)]
- self.tracks.append(CompactDiscTrack(i + 1, track_title,
- length))
- except:
- query = query_info[0]
- self.artist = query['title'][:query['title'].index(' / ')]
- self.album = query['title'][query['title'].index(' / ') + 3:]
-
- # Get track titles
- (read_status, read_info) = CDDB.read(query_info[0]['category'],
- query_info[0]['disc_id'])
- cumulative_length = 0
- for i in range(disc_id[1]):
- if i + 4 == len(disc_id):
- # We must calculate last track length different way
- length = disc_id[len(disc_id) - 1] - cumulative_length
- else:
- # Calculate track length in seconds
- length = (disc_id[i+3] - disc_id[i+2]) / 75
- cumulative_length = cumulative_length + length
-
- track_title = read_info['TTITLE' + str(i)]
- self.tracks.append(CompactDiscTrack(i + 1, track_title,
- length))
+ # query_info variable's type depends on query_status
+
+ if query_status == 200:
+ # On 200, a dictionary containing three items is returned
+ self._get_informations_from_result_element(query_info, disc_id)
+ elif query_status == 210 or query_status == 211:
+ # On 211 or 210, a list will be returned as the second item.
+ # Each element of the list will be a dictionary containing
+ # three items, exactly the same as a single 200 success return.
+ self._get_informations_from_result_element(query_info[0], disc_id)
         else:
             # No metadata found for this disc
             self.album = _("Unknown title")
             self.artist = _("Unknown artist")
             for i in range(disc_id[1]):
                 self.tracks.append(CompactDiscTrack(i + 1,
- _("Unknown track %(num)s.") % \
- {'num': str(i+1)}))
+ _("Unknown track %(num)s.") % {'num': str(i+1)}))
+
+ def _get_informations_from_result_element(self, result, disc_id):
+ """Catch any information from a CDDB result dictionnary"""
+ title = unicode(result['title'], "iso-8859-1")
+ category = unicode(result['category'], "iso-8859-1")
+ disc = unicode(result['disc_id'], "iso-8859-1")
+
+ self.artist = title[:title.index(' / ')]
+ self.album = title[title.index(' / ') + 3:]
+
+ # Get track titles
+ (status, info) = CDDB.read(category, disc)
+ cumulative_length = 0
+ for i in range(disc_id[1]):
+ if i + 4 == len(disc_id):
+ # We must calculate last track length different way
+ length = disc_id[len(disc_id) - 1] - cumulative_length
+ else:
+ # Calculate track length in seconds
+ length = (disc_id[i + 3] - disc_id[ i + 2]) / 75
+ cumulative_length = cumulative_length + length
+
+ track_title = unicode(info['TTITLE' + str(i)], "iso-8859-1")
+ self.tracks.append(CompactDiscTrack(i + 1, track_title, length))

     def get_artist(self):
         """

« Back to merge proposal