Re: PATCH: to implement toc and position shadowing in cdrom

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Waldeck Schutzer wrote:
Hi Eric,

Sorry to keep you on hold for so long. Here is my proposal for toc and position shadowing that should address those performance and functional issues we have already discussed and are summarized below. I've tested it with cdplayer and it seems to be working pretty well now. To my surprise and complete dismay it even worked the first time! It's ugly, I know, but it should do it. I don't dare to delve into the BSD parts. I'll leave these to a BSD expert.

Best,
Waldeck

Issues addressed by this patch
1. cdrom.c
1.1 Some systems/drives are very slow to read the TOC. To address this issue, we are shadowing it inside the driver.
1.2 Windows will seek while not playing, Linux will not. We are providing better compatibility with Windows by also shadowing the current position.

2. mcicda.c
Windows will play data tracks as audio silently, Linux won't play at all. With this patch we are simply avoiding playing data tracks. This is important, otherwise CDs beginning with a data track will not start playing unless you seek to the next audio track first. Since seeking to the next track while stopped was impossible before the above patch to cdrom.c, these disks would not play.
several comments:
In cdrom.diff
* in SyncCache (and some other functions), I don't understand why you test dev against the [0,26[ range. Either, dev is the driver letter (A..Z) to which the cdrom is attached, and in that case I don't see how you could use this value as a valid fd in ioctl. Or, it's the fd opened on the cdrom, and there's no reason it would be in the [0,26[ range.
Yes, the code is misleading. The dev in CDROM_Open means the drive letter, and most of the other functions take a fd in their dev function.
* for memory allocation, better use the HeapAlloc/HeapFree instead of the malloc/free. We'll be better off in the long run (especially in debugging, monitoring...)
* I wouldn't use in the cdrom_cache structure the Linux structures. As it is, it won't compile on non Linux platforms. I'd rather use the Windows one (so that it'll ease porting to other OSes). Moreover, this would allow a better coding in cdrom.c. We could split the low-level OS dependent functions on one hand (reading toc, position...) from the highest level (including caching, seeking...). (Unless we only need caching, and seeking functionalities in Linux, which I'm not sure of).

In mcicda.diff:
* you could perhaps factorize the code that computes the correct frame address (when skipping non audio tracks).
* btw, did you test what windows was returning in this case as the set of tracks on the disk. In wonder whether all the tracks (including the non audio) are returned thru the mcicda interface (or perhaps the leading non-audio are not reported).

A+

--
Eric Pouech



[Index of Archives]     [Gimp for Windows]     [Red Hat]     [Samba]     [Yosemite Camping]     [Graphics Cards]     [Wine Home]

  Powered by Linux