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

[Rocky Bernstein]

Now that the dust has settled, (and before too much more settles) Thomas'
changes have now been merged into master.  I've made only slight changes to
the Doxygen comments.

I notice that Doxygen now supports markdown
.  I will be
making a pass at some point over all of the documentation to make its
markup look nicer. (That's about all I can contribute at this point.)

With the various API changes, expect a discussion on what the next version
number will be, and how and the various library major/minior/version
numbers.

As always, we are indebted to Thomas for his diligence, thoughtfulness and
hard work.

On Wed, Jun 6, 2018 at 4:00 PM, Thomas Schmitt  wrote:

> Hi,
>
> about changeset
>   http://git.savannah.gnu.org/cgit/libcdio.git/commit/?h=
> pbatard-multiextent=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_textlsn[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_textlsn[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
>
>
>

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

[Thomas Schmitt]

Hi,

Rocky Bernstein wrote:
> Now that the dust has settled, (and before too much more settles) Thomas'
> changes have now been merged into master.

My patches belong to another discussion thread:
  "[Libcdio-devel] RFC on two CD-TEXT patches by Serge Pouliquen"
  http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg0.html
  ...
  http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg6.html
  https://savannah.gnu.org/bugs/?53928

The commit
  "Merge branch 'cdtext-list-languages'"
  
http://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=16ff26506eedd748598f2beed4aa9825d04c1c9f
not only contains my changes about CDTEXT but also new files which were
not made by me (at least not this year):

  conf9AaqoM/subs.awk
  conf9AaqoM/subs1.awk
  config_extract.sh
  example/read-disc-struct.c
  example/read-disc-struct.sh
  test/copying.iso
  test/driver/abs_path.dSYM/Contents/Info.plist
  test/driver/abs_path.dSYM/Contents/Resources/DWARF/abs_path
  test/driver/gdb.core
  test/driver/mmc_read.core
  test/iso-info.core


> With the various API changes, expect a discussion on what the next version
> number will be, and how and the various library major/minior/version
> numbers.

In my own libraries the change would count as API and ABI compatible
with no special consequences for version numbers.

Old applications will continue to work as good or bad as they did before.
If such an application runs into the shortcommings of the deprecated
function cdtext_list_languages(), then the answer to a bug report will be:
Use cdtext_list_languages_v2() and cdtext_set_language_index()
instead of cdtext_list_languages() and cdtext_select_language().

cdtext_select_language() is not deprecated generally. But it assumes a
set of language blocks with all valid and unique language codes.
cdtext_set_language_index() does not depend on such a nice situation.


Have a nice day :)

Thomas

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

[Rocky Bernstein]

On Wed, Jun 13, 2018 at 7:09 AM, Thomas Schmitt  wrote:

> Hi,
>
> Rocky Bernstein wrote:
> > Now that the dust has settled, (and before too much more settles) Thomas'
> > changes have now been merged into master.
>
> My patches belong to another discussion thread:
>   "[Libcdio-devel] RFC on two CD-TEXT patches by Serge Pouliquen"
>   http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg0.html
>   ...
>   http://lists.gnu.org/archive/html/libcdio-devel/2018-05/msg6.html
>   https://savannah.gnu.org/bugs/?53928
>
> The commit
>   "Merge branch 'cdtext-list-languages'"
>   http://git.savannah.gnu.org/cgit/libcdio.git/commit/?id=
> 16ff26506eedd748598f2beed4aa9825d04c1c9f
> not only contains my changes about CDTEXT but also new files which were
> not made by me (at least not this year):
>
>   conf9AaqoM/subs.awk
>   conf9AaqoM/subs1.awk
>   config_extract.sh
>   example/read-disc-struct.c
>   example/read-disc-struct.sh
>   test/copying.iso
>   test/driver/abs_path.dSYM/Contents/Info.plist
>   test/driver/abs_path.dSYM/Contents/Resources/DWARF/abs_path
>   test/driver/gdb.core
>   test/driver/mmc_read.core
>   test/iso-info.core
>


Hmm... this is my mistake. Thanks for pointing this out.

I'm not sure how those crept in, but I've removed all except
config_extract.sh which seems marginally useful.



>
> > With the various API changes, expect a discussion on what the next
> version
> > number will be, and how and the various library major/minior/version
> > numbers.
>
> In my own libraries the change would count as API and ABI compatible
> with no special consequences for version numbers.
>

That might be true for the semantic version number of the package, but I am
not sure this is relevant for for the libtool dynamic library version
number.  My understanding of the rules written by Nicholas Boullis is ound
in libhttp://
git.savannah.gnu.org/cgit/libcdio.git/tree/lib/driver/Makefile.am?id=16ff26506eedd748598f2beed4aa9825d04c1c9f#n30
which in part reads:

#  3. If the library source code has changed at all since the last
# update, then increment REVISION (`C:R:A' becomes `C:R+1:A').
#
#  4. If any interfaces have been added, removed, or changed since the
# last update, increment CURRENT, and set REVISION to 0.
#
#  5. If any interfaces have been added since the last public release,
# then increment AGE.
#
#  6. If any interfaces have been removed or changed since the last
# public release, then set AGE to 0. A changed interface means an
# incompatibility with previous versions.

Rule 3: library source change so we increment REVISION, but by
Rule 4: interfaces have been added so we increment CURRENT and set REVISION
to 0 and by
Rule 5: itnerfaces have been added so we increment AGE

I think right now Rule 6 doesn't apply.

So in total, the current C:R:A becomes C+1:0:A+1 for lib/driver. I believe
the other drivers are not effected, but then there's Pete's change
which are in lib/iso9660.




> Old applications will continue to work as good or bad as they did before.
> If such an application runs into the shortcommings of the deprecated
> function cdtext_list_languages(), then the answer to a bug report will be:
> Use cdtext_list_languages_v2() and cdtext_set_language_index()
> instead of cdtext_list_languages() and cdtext_select_language().
>
> cdtext_select_language() is not deprecated generally. But it assumes a
> set of language blocks with all valid and unique language codes.
> cdtext_set_language_index() does not depend on such a nice situation.
>
>
> Have a nice day :)
>
> Thomas
>
>
>

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

[Thomas Schmitt]

Hi,

Rocky Bernstein wrote:
> http://git.savannah.gnu.org/cgit/libcdio.git/tree/lib/driver/Makefile.am?id=16ff26506eedd748598f2beed4aa9825d04c1c9f#n30
> [...]
> Rule 4: interfaces have been added so we increment CURRENT and set REVISION
> to 0 and by
> Rule 5: itnerfaces have been added so we increment AGE

Yeah. That's about what i inherited with libburn too.


> So in total, the current C:R:A becomes C+1:0:A+1

Indeed. From
  libcdio_la_CURRENT = 18
  libcdio_la_REVISION = 0
  libcdio_la_AGE = 0
to
  libcdio_la_CURRENT = 19
  libcdio_la_REVISION = 0
  libcdio_la_AGE = 1

which are later used in line 172 with

  libcdio_la_MAJOR = $(shell expr $(libcdio_la_CURRENT) - $(libcdio_la_AGE))

a bit higher i read

  # compute MAJOR as CURENT - AGE; that is what is used within libtool
  # (at least on GNU/Linux systems) for the number in the SONAME.
  
So SONAME, the major lib*.so.N version number, does not change from
  18 - 0 = 18
to
  19 - 1 = 18


>  but then there's Pete's change which are in lib/iso9660.

My advise was to keep it API and ABI compatible within the limitations
of single-extent files by:
- Keeping and filling the deprecated struct members for API compatibility.
- Adding the new members to the end of the struct so that all old
  members do not move, which would spoil ABI compatibility.
Pete announced to works towards this goal.

With multi-extent files, the ill behavior of the old struct member API
might or might not change, but will stay ill. The advise towards
victims will then be to use the new multi-extent API.

-

In libburn, i increment CURRENT and AGE only once for a development cycle
and once for a release. So if i'd advise people to use the development
version, then i would now go from
   LT_CURRENT=105
   LT_AGE=101
to
   LT_CURRENT=106
   LT_AGE=102
and with the next release to
   LT_CURRENT=107
   LT_AGE=103
regardless whether 106:102 was actually used.
They all yield libburn.so.4.
Current release: libburn.so.4.101.0 (GNU/Linux)
 libburn.so.4   (FreeBSD)
 libburn.so.4.101   (OpenBSD, NetBSD)
Development: libburn.so.4[.102[.0]]
Next release:libburn.so.4[.103[.0]]

In libcdio it would now become
   libcdio_la_CURRENT = 19
   libcdio_la_AGE = 1
and at next release
   libcdio_la_CURRENT = 20
   libcdio_la_AGE = 2

I deem REVISION quite useless and always keep it at 0. 

All three parameters are handed to libtool with option -version-info.
Depending on the operating system, libtool will add one or more numbers
to the .so file name.
I had to tweak my local
  /usr/share/libtool/config/ltmain.sh
to get the intended SONAME as "major=.`expr $current - $age`" on the BSDs.
See "Building from Repositories on BSD [...]" in
  https://dev.lovelyhq.com/libburnia/web/wikis/Releases

Having a look into ./ltmain.sh of release tarball libcdio-0.94.tar.gz :
FreeBSD dynamic seems to be ok.
freebsd-elf)
  func_arith $current - $age
  major=.$func_arith_result
But i find no such case for openbsd or netbsd. I dimly remember that the
behavior of my local libtool installation was undesirable.


Have a nice day :)

Thomas