[DVDnav-discuss] libdvdread patches from my tree

[Diego Biurrun]

On Sat, Nov 12, 2011 at 12:40:58PM -0800, Erik Hovland wrote:
> Here are the patches that I intend to push to svn soon. I would like to
> test them a bit longer and get your eyeballs on these patches
> before I push. All criticism appreciated.
> 
> --- a/src/dvd_udf.c
> +++ b/src/dvd_udf.c
> @@ -611,7 +611,9 @@ static int UDFScanDir( dvd_reader_t *device, struct AD Dir, char *FileName,
>              memcpy(FileICB, &tmpICB, sizeof(tmpICB));
>              found = 1;
>            }
> -          UDFMapICB(device, tmpICB, &tmpFiletype, partition, &tmpFile);
> +          if(!UDFMapICB(device, tmpICB, &tmpFiletype, partition, &tmpFile))
> +	    return 0;
> +

Mix of tabs and spaces, should be all spaces.

> From e6a378b53b7d8d961dfe87834eb6aed9035a1bb3 Mon Sep 17 00:00:00 2001
> From: Erik Hovland <erik at hovland.org>
> Date: Mon, 26 Sep 2011 10:08:12 -0700
> Subject: [PATCH 2/8] Use a macro instead of a magic constant
> 
> The levels value is in a macro in the very next line of
> ifo_types.h. The static array declared above it should
> use it.
> ---
>  src/dvdread/ifo_types.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/dvdread/ifo_types.h b/src/dvdread/ifo_types.h
> index 7db7d34..6e5f4a1 100644
> --- a/src/dvdread/ifo_types.h
> +++ b/src/dvdread/ifo_types.h
> @@ -475,7 +475,7 @@ typedef struct {
>   * Parental Management Information Unit Table.
>   * Level 1 (US: G), ..., 7 (US: NC-17), 8
>   */
> -#define PTL_MAIT_NUM_LEVEL 8
> +#define PTL_MAIT_NUM_LEVEL 8U
>  typedef uint16_t pf_level_t[PTL_MAIT_NUM_LEVEL];

I wonder how the log message and the change match - they don't AFAICT.

> From e170cbc56844b03db6bed41fc7013265be01c1f0 Mon Sep 17 00:00:00 2001
> From: Erik Hovland <erik at hovland.org>
> Date: Mon, 10 Oct 2011 15:08:24 -0700
> Subject: [PATCH 5/8] Use NULL instead of 0, this is C kids
> 
> --- a/src/ifo_read.c
> +++ b/src/ifo_read.c
> @@ -475,7 +475,7 @@ ifo_handle_t *ifoOpenVTSI(dvd_reader_t *dvd, int title) {
>  
> -  if(ifoRead_VTS(ifofile) && ifofile->vtsi_mat)
> +  if(ifoRead_VTS(ifofile) && ifofile->vtsi_mat != NULL)
>      return ifofile;

This makes no sense except bloating the code IMO.

> From 5a6374c00f173101b1914f5862f91c4704dfbece Mon Sep 17 00:00:00 2001
> From: Erik Hovland <erik at hovland.org>
> Date: Mon, 10 Oct 2011 15:49:49 -0700
> Subject: [PATCH 6/8] Update the TODO file
> 
> ---
>  TODO |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/TODO b/TODO
> index 3fe2eed..870ffe6 100644
> --- a/TODO
> +++ b/TODO
> @@ -8,5 +10,4 @@
>     3 - fully respect user prohibitions
>  * cleanup public API and fix libtool versioning
>  * Update decoder.c with some of the more rare commands. Update already done to vmcmd.c
> -* Replace the auto* build system with a custom and clean one based on ffmpeg's (partly done)
> +* Replace the auto* build system with a custom and clean one based on ffmpeg's (mostly done)

How far along is this?  I never checked how this was done and how much
code was actually copied over.  Are you seriously interested in redoing
the build system libav style or do you just want to fix up the auto*
implementation you already have?

Diego