On Mon, 9 Mar 2015, Dave Young wrote:
> On 03/08/15 at 11:29am, Alan Stern wrote:
> > On Sun, 8 Mar 2015, Dave Young wrote:
> >
> > > I used usb cdrom emulation to play video dvd for my daughter, but I got below
> > > error:
> > >
> > > [dave@darkstar tmp]$ cat /mnt/sr1/VIDEO_TS/VTS_01_5.VOB >/dev/null
> > > cat: /mnt/sr1/VIDEO_TS/VTS_01_5.VOB: Input/output error
> > > [dave@darkstar tmp]$ dmesg|tail -1
> > > [ 3349.371857] sr1: rw=0, want=8028824, limit=4607996
> > >
> > > The assumption of cdrom size is not right, we can create data dvd large then
> > > 4G, also mkisofs can create an iso with only one blank directory, the size is
> > > less than 300 sectors. The size limit does not make sense, thus remove them.
> > >
> > > Signed-off-by: Dave Young <dyoung@xxxxxxxxxx>
> > > ---
> > > drivers/usb/gadget/function/storage_common.c | 9 ---------
> > > 1 file changed, 9 deletions(-)
> > >
> > > --- linux.orig/drivers/usb/gadget/function/storage_common.c
> > > +++ linux/drivers/usb/gadget/function/storage_common.c
> > > @@ -247,15 +247,6 @@ int fsg_lun_open(struct fsg_lun *curlun,
> > >
> > > num_sectors = size >> blkbits; /* File size in logic-block-size blocks */
> > > min_sectors = 1;
> > > - if (curlun->cdrom) {
> > > - min_sectors = 300; /* Smallest track is 300 frames */
> > > - if (num_sectors >= 256*60*75) {
> > > - num_sectors = 256*60*75 - 1;
> > > - LINFO(curlun, "file too big: %s\n", filename);
> > > - LINFO(curlun, "using only first %d blocks\n",
> > > - (int) num_sectors);
> > > - }
> > > - }
> > > if (num_sectors < min_sectors) {
> > > LINFO(curlun, "file too small: %s\n", filename);
> > > rc = -ETOOSMALL;
> >
> > NAK. This patch is wrong, for a very simple reason. You wrote:
> >
> > > I used usb cdrom emulation to play video dvd for my daughter
> >
> > and this demonstrates the error quite plainly. You can't use _CD_
> > emulation to imitate a _DVD_ -- they are different sorts of media.
> > Just think of what happens when you try playing a DVD on a CD player.
> >
> > A more suitable approach would be to add DVD emulation to the driver.
> >
>
> They are both iso9660 images, aren't they? So from data image point
> of view there's no difference, it is not worth to create another mode
> for dvd data.
There's more to emulation than just the image. We have to emulate the
hardware's response to all the applicable commands. CD players have a
different command set from DVD players (or at least, different from DVD
players when a DVD is inserted). For example, see the definition in
the MSF format for READ TOC command.
> We should not emulate cd drive to support different cd media type,
> it is far more better to support just iso9660 volume no matter how
> large the size is as long as it is fit for iso9660 spec.
>
> OTOH, the size limitation is a bug:
> * isofs can be less than 300 sectors, the 300 sectors limitation is for
> music cd I think. Try mkisofs a blank directory and burn it.
You're thinking about this the wrong way. We aren't emulating an
iso9660 image; we are emulating a CD player. (In principle we could
even emulate an audio CD, but the driver doesn't support that.)
> * There's 99 minutes dics even for cdrom, see:
> http://en.wikipedia.org/wiki/CD-R
> If we code to support different size discs, it will looks like a wrong way.
Look at the code you want to remove:
> > > - if (num_sectors >= 256*60*75) {
That's 75 sectors/second * 60 seconds/minute * 256 minutes. So the
driver already supports up to 256 minutes, which is way beyond the 99
minutes required by the spec.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
