[MPlayer-dev-eng] [PATCH] -identify audio CD track length

[Guillaume POIRIER]

Hi,

On 9/8/05, kiriuja <mplayer-patches at en-directo.net> wrote:
> Prints ID_CDDA_TRACK_N_LENGTH=<min>:<sec>:<frame> for each track
> on an audio CD when -identify is given.

This hunk doesn't seem too obvious to me, so I'd like you to comment it a bit:

+  i = cdd_identify(p->device);
+  if(i >= 0) {
   if(strncmp(st->url,"cddb",4) == 0) {
     i = cddb_resolve(p->device, &xmcd_file);
     if(i == 0) {
@@ -119,6 +122,7 @@ static int open_cdda(stream_t *st,int m,
       free(xmcd_file);
     }
   }
+  }

I don't understand why calling cdd_identify(p->device), which, as its
names states, just "identifies" p->device gives you information to
skip a part of the code.
Maybe the existing code was needlessly called already, and your fixed
this, which is good, but needs to go in a seperate patch IMHO,
otherwise you need to add a comment so that this change makes sense to
someone like me.

+	if (identify)
+	{
+		int i, min, sec, frame;
+		cdtoc_last_track = read_toc(dev);
+		if (cdtoc_last_track < 0) {
+			printf("Failed to open %s device.\n", dev);
+			return -1;
+		}

Aren't all messages supposed to use mp_msg() ? The existing code does
use one printf at least, but that maybe needs to be fixed later.

Guillaume
-- 
Reading doesn't hurt, really!
 -- Dominik 'Rathann' Mierzejewski