Re: [PATCH 2/2] Add 0x49 descriptor support

See review remarks below:

On Mon, May 21, 2012 at 5:09 PM, Roberto Corno <[email protected]> wrote:
> ---
>  src/Makefile.am         |    3 +-
>  src/descriptors/dr.h    |    1 +
>  src/descriptors/dr_49.c |  122 +++++++++++++++++++++++++++++++++++++++++++++++
>  src/descriptors/dr_49.h |  106 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 231 insertions(+), 1 deletions(-)
>  create mode 100644 src/descriptors/dr_49.c
>  create mode 100644 src/descriptors/dr_49.h
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index ec07efa..2b78ee8 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -38,6 +38,7 @@ pkginclude_HEADERS = dvbpsi.h psi.h descriptor.h demux.h custom.h \
>                      descriptors/dr_45.h \
>                      descriptors/dr_47.h \
>                      descriptors/dr_48.h \
> +                     descriptors/dr_49.h \
>                      descriptors/dr_4d.h \
>                      descriptors/dr_4e.h \
>                      descriptors/dr_52.h \
> @@ -64,7 +65,6 @@ descriptors_src = descriptors/dr_02.c \
>                   descriptors/dr_0d.c \
>                   descriptors/dr_0e.c \
>                   descriptors/dr_0f.c \
> -                  descriptors/dr_40.c \
>                   descriptors/dr_41.c \
>                   descriptors/dr_42.c \
>                   descriptors/dr_43.c \
> @@ -72,6 +72,7 @@ descriptors_src = descriptors/dr_02.c \
>                   descriptors/dr_45.c \
>                   descriptors/dr_47.c \
>                   descriptors/dr_48.c \
> +                  descriptors/dr_49.c \
>                   descriptors/dr_4d.c \
>                   descriptors/dr_4e.c \
>                   descriptors/dr_52.c \
> diff --git a/src/descriptors/dr.h b/src/descriptors/dr.h
> index 5e71541..c866135 100644
> --- a/src/descriptors/dr.h
> +++ b/src/descriptors/dr.h
> @@ -54,6 +54,7 @@
>  #include "dr_45.h"
>  #include "dr_47.h"
>  #include "dr_48.h"
> +#include "dr_49.h"
>  #include "dr_4d.h"
>  #include "dr_4e.h"
>  #include "dr_52.h"
> diff --git a/src/descriptors/dr_49.c b/src/descriptors/dr_49.c
> new file mode 100644
> index 0000000..8e9cb79
> --- /dev/null
> +++ b/src/descriptors/dr_49.c
> @@ -0,0 +1,122 @@
> +/*
> + * dr_49.c
> + * Copyright (C) 2001-2011 VideoLAN

Year is 2012.

> + *
> + * Authors: rcorno (Nov 22, 2011)
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + *****************************************************************************/
> +
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdbool.h>
> +#include <string.h>
> +
> +#if defined(HAVE_INTTYPES_H)
> +#include <inttypes.h>
> +#elif defined(HAVE_STDINT_H)
> +#include <stdint.h>
> +#endif
> +
> +#include "../dvbpsi.h"
> +#include "../dvbpsi_private.h"
> +#include "../descriptor.h"
> +
> +#include "dr_49.h"
> +
> +/*****************************************************************************
> + * dvbpsi_DecodeCountryAvailability
> + *****************************************************************************/
> +dvbpsi_country_availability_dr_t* dvbpsi_DecodeCountryAvailability(
> +                                        dvbpsi_descriptor_t * p_descriptor)
> +{
> +       dvbpsi_country_availability_dr_t * p_decoded;
> +       int i;

Identation.

> +
> +    /* Check the tag */
> +    if (p_descriptor->i_tag != 0x49)
> +        return NULL;
> +
> +    /* Don't decode twice */
> +    if (p_descriptor->p_decoded)
> +        return p_descriptor->p_decoded;
> +
> +    /* Allocate memory */
> +    p_decoded = (dvbpsi_country_availability_dr_t*)calloc(1, sizeof(dvbpsi_country_availability_dr_t));
> +    if (!p_decoded)
> +        return NULL;
> +
> +    /* Decode data and check the length */
> +    if((p_descriptor->i_length < 1) || ((p_descriptor->i_length-1) % 3 != 0))
> +    {
> +      free(p_decoded);
> +      return NULL;
> +    }

Check length before allocating memory.

> +    p_decoded->i_code_count= (p_descriptor->i_length-1) / 3;

Check if i_code_count is below 84.

> +    p_decoded->b_country_availability_flag = p_descriptor->p_data[0] & 0x80;
> +    i=0;
> +    while( i < p_decoded->i_code_count ) {
> +       p_decoded->code[i].iso_639_code[0] = p_descriptor->p_data[1+i*3];
> +       p_decoded->code[i].iso_639_code[1] = p_descriptor->p_data[2+i*3];
> +       p_decoded->code[i].iso_639_code[2] = p_descriptor->p_data[3+i*3];
> +       i++;
> +    }
> +
> +    p_descriptor->p_decoded = (void*)p_decoded;
> +
> +    return p_decoded;
> +}
> +
> +
> +/*****************************************************************************
> + * dvbpsi_GenCountryAvailabilityDr
> + *****************************************************************************/
> +dvbpsi_descriptor_t * dvbpsi_GenCountryAvailabilityDr(
> +                                               dvbpsi_country_availability_dr_t * p_decoded,
> +                                        bool b_duplicate)

Indentation

> +{
> +    /* Create the descriptor */
> +    dvbpsi_descriptor_t * p_descriptor =
> +            dvbpsi_NewDescriptor(0x83, 1+p_decoded->i_code_count*3, NULL);
> +
> +    if (!p_descriptor)
> +        return NULL;
> +
> +    /* Encode data */
> +    p_descriptor->p_data[0] = (p_decoded->b_country_availability_flag)?0x80:0x00;
> +    int i = 0;

Check if i_code_count is below 84.

> +    while( i < p_decoded->i_code_count ) {
> +        p_descriptor->p_data[1+i*3] = p_decoded->code[i].iso_639_code[0];
> +        p_descriptor->p_data[2+i*3] = p_decoded->code[i].iso_639_code[1];
> +        p_descriptor->p_data[3+i*3] = p_decoded->code[i].iso_639_code[2];
> +      i++;
> +    }
> +
> +    if (b_duplicate)
> +    {
> +        /* Duplicate decoded data */
> +       dvbpsi_country_availability_dr_t * p_dup_decoded =
> +                (dvbpsi_country_availability_dr_t*)calloc(1, sizeof(dvbpsi_country_availability_dr_t));
> +        if(p_dup_decoded)
> +            memcpy(p_dup_decoded, p_decoded, sizeof(dvbpsi_country_availability_dr_t));
> +
> +        p_descriptor->p_decoded = (void*)p_dup_decoded;

Use new helper function here:

        p_descriptor->p_decoded =
                dvbpsi_DuplicateDecodedDescriptor(p_descriptor->p_decoded,

sizeof(dvbpsi_country_availability_dr_t));
> +    }
> +
> +    return p_descriptor;
> +}
> diff --git a/src/descriptors/dr_49.h b/src/descriptors/dr_49.h
> new file mode 100644
> index 0000000..fa73f6c
> --- /dev/null
> +++ b/src/descriptors/dr_49.h
> @@ -0,0 +1,106 @@
> +/*****************************************************************************
> + * dr_49.h
> + * Copyright (C) 2001-2011 VideoLAN

Year is 2011-2012

> + *
> + * Authors: rcorno (Nov 22, 2011)
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + *****************************************************************************/
> +
> +/*!
> + * \file <dr_49.h>
> + * \author Corno Roberto <[email protected]>
> + * \brief Application interface for the DVB "country availability"
> + * descriptor decoder and generator.
> + *
> + * Application interface for the DVB "country availability" descriptor
> + * decoder and generator. This descriptor's definition can be found in
> + * ETSI EN 300 468 section 6.2.10.
> + */
> +
> +#ifndef DR_49_H_
> +#define DR_49_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +typedef uint8_t iso_639_language_code_t[3]; /*!< ISO639 three letter language codes */
> +
> +/*****************************************************************************
> + * dvbpsi_country_availability_dr_t
> + *****************************************************************************/
> +/*!
> + * \struct dvbpsi_country_availability_dr_t
> + * \brief "country availability" descriptor structure.
> + *
> + * This structure is used to store a decoded "country availability"
> + * descriptor. (ETSI EN 300 468 section 6.2.10).
> + */
> +/*!
> + * \typedef struct dvbpsi_country_availability_dr_s dvbpsi_country_availability_dr_t
> + * \brief dvbpsi_country_availability_dr_t type definition.
> + */
> +typedef struct dvbpsi_country_availability_dr_s
> +{
> +  bool                 b_country_availability_flag;    /*!< country availability flag */
> +  uint8_t       i_code_count;                          /*!< length of the i_iso_639_code
> +                                               array */

indentation.

> +  struct {
> +    iso_639_language_code_t  iso_639_code;
> +  } code[84];                                  /*!< ISO_639_language_code */
> +
> +} dvbpsi_country_availability_dr_t;
> +
> +/*****************************************************************************
> + * dvbpsi_DecodeCountryAvailabilityDr
> + *****************************************************************************/
> +/*!
> + * \fn dvbpsi_country_availability_dr_t * dvbpsi_DecodeCountryAvailability(
> +                                        dvbpsi_descriptor_t * p_descriptor)
> + * \brief "country availability" descriptor decoder.
> + * \param p_descriptor pointer to the descriptor structure
> + * \return a pointer to a new "country availability" descriptor structure
> + * which contains the decoded data.
> + */
> +dvbpsi_country_availability_dr_t* dvbpsi_DecodeCountryAvailability(
> +                                        dvbpsi_descriptor_t * p_descriptor);
> +
> +
> +/*****************************************************************************
> + * dvbpsi_GenCountryAvailabilityDr
> + *****************************************************************************/
> +/*!
> + * \fn dvbpsi_descriptor_t * dvbpsi_GenCountryAvailabilityDr(
> +                        dvbpsi_country_availability_dr_t * p_decoded, int b_duplicate)
> + * \brief "country availability" descriptor generator.
> + * \param p_decoded pointer to a decoded "country availability" descriptor
> + * structure
> + * \param b_duplicate if non zero then duplicate the p_decoded structure into
> + * the descriptor
> + * \return a pointer to a new descriptor structure which contains encoded data.
> + */
> +dvbpsi_descriptor_t * dvbpsi_GenCountryAvailabilityDr(
> +                                               dvbpsi_country_availability_dr_t * p_decoded,
> +                                        bool b_duplicate);

indentation

> +
> +#ifdef __cplusplus
> +};
> +#endif
> +
> +#else
> +#error "Multiple inclusions of dr_49.h"
> +#endif /* DR_49_H_ */
> --
> 1.7.5.4
>
> _______________________________________________
> libdvbpsi-devel mailing list
> [email protected]
> http://mailman.videolan.org/listinfo/libdvbpsi-devel

Kind regards,

Jean-Paul Saman
_______________________________________________
libdvbpsi-devel mailing list
[email protected]
http://mailman.videolan.org/listinfo/libdvbpsi-devel