Re: can't read DVD (under 2.4.[12] & 2.2.17)

[Jens Axboe <axboe@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Wed, Mar 07 2001, Derek Fawcus wrote:
> > > hdd: packet command error: status=0x51 { DriveReady SeekComplete Error }
> > > hdd: packet command error: error=0x50
> > > ATAPI device hdd:
> > >   Error: Illegal request -- (Sense key=0x05)
> > >   Invalid field in command packet -- (asc=0x24, ascq=0x00)
> > >   The failed "Send DVD Structure" packet command was:
> > >   "ad 00 00 00 00 00 02 00 00 54 00 00 "
> > > Could not read Physical layer 2
> > > Copyright: CPST=1, RMI=0xfd
> > 
> > I don't know the program you mention, but it's definitely buggy. It
> > sets byte 6 to 0x02 which is not valid at all.
> 
> The program is attempting to read from the third layer!  Thats why it
> fails.  This value gets set from the .layer_num element supplied
> in the struct dvd_physical supplied to the ioctl call.
> 
> > Byte 7 is the format code, but 0x02 is reserved there too. Who wrote
> > this program? Tell him it's buggy, it's not the driver.
> 
> Originally me (a couple of years ago),  and I'm not supprised at it 
> being buggy,  it was a simple program to explore a couple of the
> ioctl calls.  I never got particularly sensible data from a multi-layer
> DVD (maybe the firmware in my drives is dodgy).

The report sense was buggy, and that fooled me. The cdb is not wrong
at all, apart from the length which you mention further on. It's a
READ_DVD_STRUCTURE command, not a SEND_DVD_STRUCTURE.

> the original code was something like:
> 
>   dvd_struct d;
>   int layer, layers = 4;
> 
>   d.physical.type = DVD_STRUCT_PHYSICAL;
> 
>   for (layer = 0; layer < layers; ++layer) {
>     d.physical.layer_num = layer;
> 
>     ioctl(fd, DVD_READ_STRUCT, &d);
> 
>     printf("Layer %d[%d]\n", layer, layers);
>     display data in d.physical.layer[layer]
> 
>     layers = d.physical.layer[layer].nlayers + 1;
>   }

Much the same today, the program was forwarded to me ;-)

> The way I interpreted this API is that one requests the layer number to
> read the data from,  and that index in the array would be filled in with
> data.  However this seems a bit pointless - if only one layers worth of
> data will be returned at a time,  then why have an array of 4 entries?

Really good question, I sent this patch in the private thread between
me and Pozsar just in case the length is what the drive complains about.

-- 
Jens Axboe


[-- Attachment #2: dvd-read-phys-1 --]
[-- Type: text/plain, Size: 410 bytes --]

--- /opt/kernel/linux-2.4.3-pre2/drivers/cdrom/cdrom.c	Thu Feb 22 14:55:22 2001
+++ drivers/cdrom/cdrom.c	Wed Mar  7 22:25:14 2001
@@ -1172,7 +1172,7 @@
 static int dvd_read_physical(struct cdrom_device_info *cdi, dvd_struct *s)
 {
 	int ret, i;
-	u_char buf[4 + 4 * 20], *base;
+	u_char buf[20], *base;
 	struct dvd_layer *layer;
 	struct cdrom_generic_command cgc;
 	struct cdrom_device_ops *cdo = cdi->ops;