Re: [Libcdio-devel] [RFC/PATCH] add multi extent ISO9660 file support to libcdio

[Thomas Schmitt]

Hi,

about changeset
  
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=pbatard-multiextent&id=c93341a28bba1842d7df89d784951eccc18050ff

------------------------------------------------------------------------

I think that is unfortunate to break the old examples (and the API).

Maybe we regret it later, but how about staying API compatible by
keeping the old struct members and adding new ones for possible more
extents ?

  lsn_t lsn; /**< start logical sector number */
  uint32_t size; /**< size of first extent in bytes */
  uint32_t secsize; /**< number of sectors allocated for first extent */

  ... other old struct members ...

  uint64_t total_size;  /**< combined size of all extents in bytes */
  uint32_t extents;     /**< number of extents */
           /**v start logical sector number of extents after the first one */
  lsn_t    extlsn[ISO_MAX_MULTIEXTENT - 1];
           /**v sizes of extents after the first one */
  uint32_t extsize[ISO_MAX_MULTIEXTENT - 1];
           /**v number of sectors allocated for extlsn[] */
  uint32_t extsecsize[ISO_MAX_MULTIEXTENT - 1];
};

Old API and ABI would be preserved.

Old application code would continue to work for single-extent files.
Multi-extent files would be seen only once (not multiple times) but of
course wrongly with only the size of the first extent.

For the API we could define a call which presents the extents in
a neat array, thus eliminating the need for distinct code handling the
first extent and other code for handling the following extents.

Or: If we are willing to waste 12 bytes per struct, then we could put
the first extent's info into the [0] elements of the new arrays.

  /* Legacy API. Deprecated. Use the Multi-Extent API. */
  lsn_t lsn; /**< start logical sector number */
  uint32_t size; /**< size of first extent in bytes */
  uint32_t secsize; /**< number of sectors allocated for first extent */

  /* Multi-Extent API */
  uint64_t total_size;  /**< combined size of all extents in bytes */
  uint32_t extents;     /**< number of extents */
           /**v start logical sector number of extents */
  lsn_t    extlsn[ISO_MAX_MULTIEXTENT];
           /**v sizes of extents */
  uint32_t extsize[ISO_MAX_MULTIEXTENT];
           /**v number of sectors allocated for extlsn[] */
  uint32_t extsecsize[ISO_MAX_MULTIEXTENT];

This would spare the plight to separately handle first and further
extents in the internal code of libcdio. (And we'd need no extra API
call which hands out a neat array.)

----------------------------------------------------------------------

I wonder why there is

  uint32_t secsize; /**< number of sectors allocated for first extent */

It gets set in lib/iso9660/iso9660_fs.c by

  p_stat->secsize = _cdio_len2blocks (p_stat->size, ISO_BLOCKSIZE);

which computes the number of blocks of the extent from the number
of bytes.
We could define macros or functions
  CDIO_EXTENT_BLOCKS(size) ... to give the number of blocks
  CDIO_EXTENT_BBYTES(size) ... to give the rounded-up number of bytes
and use them where currently
  secsize[...]
  secsize[...] * ISO_BLOCKSIZE
are used.

Throwing out extsecsize[] would save more bytes than we'd waste by
storing the info of the first extent in the legacy part and also in
the new arrays extlsn[] and extsize[].

----------------------------------------------------------------------

The new API exposes ISO_MAX_MULTIEXTENT to the user.
This will break the ABI if we ever increase the number.

But currently i have no good proposal which avoids the need for a
destructor function of iso9660_stat_t.

----------------------------------------------------------------------

Shouldn't example/isolist.c show the multiple extents as one single
file ?
Maybe with a column which tells the number of extents, and one that
tells whether the bytes of all extents form a single array without
gaps.

----------------------------------------------------------------------

Have a nice day :)

Thomas