* [9front] [patch] cdfs handle block sizes correctly
@ 2022-10-01 10:34 Arne Meyer
2022-10-01 21:20 ` ori
0 siblings, 1 reply; 6+ messages in thread
From: Arne Meyer @ 2022-10-01 10:34 UTC (permalink / raw)
To: 9front
The Readblock enum does not work when you try to read audio cds. 4 cdda blocks are lager than 8192 and the command fails (at least when using an usb cd drive). This sets the block count according to the track block size. With this and the fix for libdisk I can rip audio cds on my el cheapo usb dvd drive. Tested with "Midnight Oil - Blue Sky Mining"
Greetings,
Arne
diff e938acc8ff64a3cbbd6ef7ba88f83e3f03ede681 uncommitted
--- a/sys/src/cmd/cdfs/dat.h
+++ b/sys/src/cmd/cdfs/dat.h
@@ -133,10 +133,11 @@
DVDNblock = 16, /* DVD ECC block is 16 sectors */
BDNblock = 32, /* BD ECC block (`cluster') is 32 sectors */
/*
- * make a single transfer fit in a 9P rpc. if we don't do this,
- * remote access (e.g., via /mnt/term/dev/sd*) fails mysteriously.
+ * number of blocks read/written must fit in this. if we don't do this,
+ * remote access (e.g., via /mnt/term/dev/sd* or nusb/disk) fails mysteriously.
+ * see /sys/src/9/port/devmnt.c MAXRPC.
*/
- Readblock = 8192/BScdrom,
+ Maxrpc = 8192,
};
typedef struct Buf Buf;
--- a/sys/src/cmd/cdfs/mmc.c
+++ b/sys/src/cmd/cdfs/mmc.c
@@ -1171,7 +1171,7 @@
o->track = &drive->track[trackno];
o->nchange = drive->nchange;
o->omode = OREAD;
- o->buf = bopen(mmcread, OREAD, o->track->bs, Readblock);
+ o->buf = bopen(mmcread, OREAD, o->track->bs, Maxrpc/o->track->bs);
o->buf->otrack = o;
aux->nropen++;
@@ -1395,7 +1395,7 @@
o->nchange = drive->nchange;
o->omode = OWRITE;
o->track = t;
- o->buf = bopen(mmcwrite, OWRITE, bs, Readblock);
+ o->buf = bopen(mmcwrite, OWRITE, bs, Maxrpc/bs);
o->buf->otrack = o;
aux->nwopen++;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] [patch] cdfs handle block sizes correctly
2022-10-01 10:34 [9front] [patch] cdfs handle block sizes correctly Arne Meyer
@ 2022-10-01 21:20 ` ori
2022-10-02 18:56 ` Arne Meyer
0 siblings, 1 reply; 6+ messages in thread
From: ori @ 2022-10-01 21:20 UTC (permalink / raw)
To: 9front
Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> The Readblock enum does not work when you try to read audio cds. 4
> cdda blocks are lager than 8192 and the command fails (at least when
> using an usb cd drive). This sets the block count according to the
> track block size. With this and the fix for libdisk I can rip audio
> cds on my el cheapo usb dvd drive. Tested with "Midnight Oil - Blue
> Sky Mining"
> Greetings,
> Arne
>
> typedef struct Buf Buf;
> --- a/sys/src/cmd/cdfs/mmc.c
> +++ b/sys/src/cmd/cdfs/mmc.c
> @@ -1171,7 +1171,7 @@
> o->track = &drive->track[trackno];
> o->nchange = drive->nchange;
> o->omode = OREAD;
> - o->buf = bopen(mmcread, OREAD, o->track->bs, Readblock);
> + o->buf = bopen(mmcread, OREAD, o->track->bs, Maxrpc/o->track->bs);
these changes smell funny to me; if the block size is larger than Maxrpc, we
set nblock to 0. Looking at bread in buf.c, it seems like we'd simplly end up
trying to read 0 bytes out of these large buffers.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] [patch] cdfs handle block sizes correctly
2022-10-01 21:20 ` ori
@ 2022-10-02 18:56 ` Arne Meyer
2022-10-02 20:04 ` ori
0 siblings, 1 reply; 6+ messages in thread
From: Arne Meyer @ 2022-10-02 18:56 UTC (permalink / raw)
To: 9front
[-- Attachment #1: Type: text/plain, Size: 1513 bytes --]
The old Readblock value is fine for data tracks, because 4 2048 byte blocks fit in the 8192 byte RPCMAX. But cdda blocks are 2352 bytes and 4 of those don't fit into 8192 bytes and stuff breaks. If I read the code correctly my change should set the number of blocks to 4 for data tracks and to 3 for everything else.
I've attached the patch to this mail to preserve whitespace.
> ori@eigenstate.org hat am 01.10.2022 21:20 GMT geschrieben:
>
>
> Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> > The Readblock enum does not work when you try to read audio cds. 4
> > cdda blocks are lager than 8192 and the command fails (at least when
> > using an usb cd drive). This sets the block count according to the
> > track block size. With this and the fix for libdisk I can rip audio
> > cds on my el cheapo usb dvd drive. Tested with "Midnight Oil - Blue
> > Sky Mining"
> > Greetings,
> > Arne
>
>
> >
> > typedef struct Buf Buf;
> > --- a/sys/src/cmd/cdfs/mmc.c
> > +++ b/sys/src/cmd/cdfs/mmc.c
> > @@ -1171,7 +1171,7 @@
> > o->track = &drive->track[trackno];
> > o->nchange = drive->nchange;
> > o->omode = OREAD;
> > - o->buf = bopen(mmcread, OREAD, o->track->bs, Readblock);
> > + o->buf = bopen(mmcread, OREAD, o->track->bs, Maxrpc/o->track->bs);
>
> these changes smell funny to me; if the block size is larger than Maxrpc, we
> set nblock to 0. Looking at bread in buf.c, it seems like we'd simplly end up
> trying to read 0 bytes out of these large buffers.
[-- Attachment #2: cdfs.patch --]
[-- Type: application/octet-stream, Size: 1245 bytes --]
diff e938acc8ff64a3cbbd6ef7ba88f83e3f03ede681 uncommitted
--- a/sys/src/cmd/cdfs/dat.h
+++ b/sys/src/cmd/cdfs/dat.h
@@ -133,10 +133,11 @@
DVDNblock = 16, /* DVD ECC block is 16 sectors */
BDNblock = 32, /* BD ECC block (`cluster') is 32 sectors */
/*
- * make a single transfer fit in a 9P rpc. if we don't do this,
- * remote access (e.g., via /mnt/term/dev/sd*) fails mysteriously.
+ * number of blocks read/written must fit in this. if we don't do this,
+ * remote access (e.g., via /mnt/term/dev/sd* or nusb/disk) fails mysteriously.
+ * see /sys/src/9/port/devmnt.c MAXRPC.
*/
- Readblock = 8192/BScdrom,
+ Maxrpc = 8192,
};
typedef struct Buf Buf;
--- a/sys/src/cmd/cdfs/mmc.c
+++ b/sys/src/cmd/cdfs/mmc.c
@@ -1171,7 +1171,7 @@
o->track = &drive->track[trackno];
o->nchange = drive->nchange;
o->omode = OREAD;
- o->buf = bopen(mmcread, OREAD, o->track->bs, Readblock);
+ o->buf = bopen(mmcread, OREAD, o->track->bs, Maxrpc/o->track->bs);
o->buf->otrack = o;
aux->nropen++;
@@ -1395,7 +1395,7 @@
o->nchange = drive->nchange;
o->omode = OWRITE;
o->track = t;
- o->buf = bopen(mmcwrite, OWRITE, bs, Readblock);
+ o->buf = bopen(mmcwrite, OWRITE, bs, Maxrpc/bs);
o->buf->otrack = o;
aux->nwopen++;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] [patch] cdfs handle block sizes correctly
2022-10-02 18:56 ` Arne Meyer
@ 2022-10-02 20:04 ` ori
2022-10-22 16:20 ` Arne Meyer
0 siblings, 1 reply; 6+ messages in thread
From: ori @ 2022-10-02 20:04 UTC (permalink / raw)
To: 9front
Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> The old Readblock value is fine for data tracks, because 4 2048 byte blocks fit in the 8192 byte RPCMAX. But cdda blocks are 2352 bytes and 4 of those don't fit into 8192 bytes and stuff breaks. If I read the code correctly my change should set the number of blocks to 4 for data tracks and to 3 for everything else.
Ah, I misunderstood you -- I thought you were saying you had disks
where the blocksize was larger than RPCMAX (8k).
I'll take another look.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] [patch] cdfs handle block sizes correctly
2022-10-02 20:04 ` ori
@ 2022-10-22 16:20 ` Arne Meyer
2022-10-22 18:43 ` ori
0 siblings, 1 reply; 6+ messages in thread
From: Arne Meyer @ 2022-10-22 16:20 UTC (permalink / raw)
To: 9front
ping?
> ori@eigenstate.org hat am 02.10.2022 20:04 GMT geschrieben:
>
>
> Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> > The old Readblock value is fine for data tracks, because 4 2048 byte blocks fit in the 8192 byte RPCMAX. But cdda blocks are 2352 bytes and 4 of those don't fit into 8192 bytes and stuff breaks. If I read the code correctly my change should set the number of blocks to 4 for data tracks and to 3 for everything else.
>
> Ah, I misunderstood you -- I thought you were saying you had disks
> where the blocksize was larger than RPCMAX (8k).
>
> I'll take another look.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [9front] [patch] cdfs handle block sizes correctly
2022-10-22 16:20 ` Arne Meyer
@ 2022-10-22 18:43 ` ori
0 siblings, 0 replies; 6+ messages in thread
From: ori @ 2022-10-22 18:43 UTC (permalink / raw)
To: 9front
Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> ping?
>
> > ori@eigenstate.org hat am 02.10.2022 20:04 GMT geschrieben:
> >
> >
> > Quoth Arne Meyer <meyer.arne83@netcologne.de>:
> > > The old Readblock value is fine for data tracks, because 4 2048 byte blocks fit in the 8192 byte RPCMAX. But cdda blocks are 2352 bytes and 4 of those don't fit into 8192 bytes and stuff breaks. If I read the code correctly my change should set the number of blocks to 4 for data tracks and to 3 for everything else.
> >
> > Ah, I misunderstood you -- I thought you were saying you had disks
> > where the blocksize was larger than RPCMAX (8k).
> >
> > I'll take another look.
ponged and applied; thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-22 18:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-01 10:34 [9front] [patch] cdfs handle block sizes correctly Arne Meyer
2022-10-01 21:20 ` ori
2022-10-02 18:56 ` Arne Meyer
2022-10-02 20:04 ` ori
2022-10-22 16:20 ` Arne Meyer
2022-10-22 18:43 ` ori
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).