[libdvbpsi-devel] [vlc-devel] [PATCH] Add 0x40 descriptor support
Jean-Paul Saman
jpsaman at videolan.org
Fri Dec 2 13:24:27 CET 2011
Hi,
This patch belongs on the libdvbpsi-devel mailinglist
Thanks for the patch.
My review comments are in between your patch.
On Tue, Nov 22, 2011 at 2:37 PM, Roberto Corno <corno.roberto at gmail.com> wrote:
> ---
> src/descriptors/dr.h | 1 +
> src/descriptors/dr_40.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
> src/descriptors/dr_40.h | 86 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 177 insertions(+), 0 deletions(-)
> create mode 100644 src/descriptors/dr_40.c
> create mode 100644 src/descriptors/dr_40.h
>
> diff --git a/src/descriptors/dr.h b/src/descriptors/dr.h
> index faf318a..93b27ee 100644
> --- a/src/descriptors/dr.h
> +++ b/src/descriptors/dr.h
> @@ -46,6 +46,7 @@
> #include "dr_0d.h"
> #include "dr_0e.h"
> #include "dr_0f.h"
> +#include "dr_40.h"
> #include "dr_42.h"
> #include "dr_43.h"
> #include "dr_44.h"
> diff --git a/src/descriptors/dr_40.c b/src/descriptors/dr_40.c
> new file mode 100644
> index 0000000..0ae431e
> --- /dev/null
> +++ b/src/descriptors/dr_40.c
> @@ -0,0 +1,90 @@
> +
> +
Missing License, Author and or Copyright notice..
See other descriptors for accepted values.
> +
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <stdlib.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_40.h"
> +
> +/*****************************************************************************
> + * dvbpsi_DecodeNetworkNameDr
> + *****************************************************************************/
> +dvbpsi_network_name_dr_t* dvbpsi_DecodeNetworkNameDr(
> + dvbpsi_descriptor_t * p_descriptor) {
Coding style violation. You should put the '{' bracket on the next line.
> + dvbpsi_network_name_dr_t * p_decoded;
> +
> + /* Check the tag */
> + if(p_descriptor->i_tag != 0x40)
A space between if and ( is wanted here.
> + {
> + DVBPSI_ERROR_ARG("dr_40 decoder", "bad tag (0x%x)", p_descriptor->i_tag);
> + return NULL;
> + }
> + /* Don't decode twice */
> + if(p_descriptor->p_decoded)
> + return p_descriptor->p_decoded;
> + /* Allocate memory */
> + p_decoded =
> + (dvbpsi_network_name_dr_t*)malloc(sizeof(dvbpsi_network_name_dr_t));
Use calloc(1, sizeof(dvbpsi_network_name_dr_t)) here
> + if(!p_decoded)
> + {
> + DVBPSI_ERROR("dr_40 decoder", "out of memory");
> + return NULL;
> + }
> + /* Decode data */
> + p_decoded->i_name_length = p_descriptor->i_length;
check if p_descriptor->i_length does not exceed 255
> + if(p_decoded->i_name_length)
> + memcpy(p_decoded->i_name_byte,
> + p_descriptor->p_data,
> + p_decoded->i_name_length);
Are you sure p_descriptor->p_data is not NULL ??
> +
> + p_descriptor->p_decoded = (void*)p_decoded;
> +
> + return p_decoded;
> +}
> +
> +
> +/*****************************************************************************
> + * dvbpsi_GenNetworkNameDr
> + *****************************************************************************/
> +dvbpsi_descriptor_t * dvbpsi_GenNetworkNameDr(
> + dvbpsi_network_name_dr_t * p_decoded,
> + int b_duplicate) {
> + /* Create the descriptor */
> + dvbpsi_descriptor_t * p_descriptor =
> + dvbpsi_NewDescriptor(0x40, p_decoded->i_name_length, NULL);
> +
> + if(p_descriptor)
> + {
> + /* Encode data */
> + if(p_decoded->i_name_length)
> + memcpy(p_descriptor->p_data,
> + p_decoded->i_name_byte,
> + p_decoded->i_name_length);
check p_decoded->i_name_length to not be larger then 255.
> +
> + if(b_duplicate)
> + {
> + /* Duplicate decoded data */
> + dvbpsi_network_name_dr_t * p_dup_decoded =
> + (dvbpsi_network_name_dr_t*)malloc(sizeof(dvbpsi_network_name_dr_t));
> + if(p_dup_decoded)
Use calloc(1, sizeof(dvbpsi_network_name_dr_t)); here.
Check for p_dub_decoded is not NULL, if it is clean up and return
NULL, else do the following below.
> + memcpy(p_dup_decoded, p_decoded, sizeof(dvbpsi_network_name_dr_t));
> +
> + p_descriptor->p_decoded = (void*)p_dup_decoded;
> + }
> + }
> +
> + return p_descriptor;
> +}
> diff --git a/src/descriptors/dr_40.h b/src/descriptors/dr_40.h
> new file mode 100644
> index 0000000..4c9c8ee
> --- /dev/null
> +++ b/src/descriptors/dr_40.h
> @@ -0,0 +1,86 @@
> +/*
> + * dr_40.h
> + *
> + * Created on: Nov 22, 2011
> + * Author: rcorno
> + */
Missing License, Author and or Copyright notice..
> +
> +/*!
> + * \file <dr_40.h>
> + * \author Corno Roberto <corno.roberto at gmail.com>
> + * \brief Application interface for the DVB "network name"
> + * descriptor decoder and generator.
> + *
> + * Application interface for the DVB "network name" descriptor
> + * decoder and generator. This descriptor's definition can be found in
> + * ETSI EN 300 468 section 6.2.27.
> + */
> +
> +#ifndef DR_40_H_
> +#define DR_40_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/*****************************************************************************
> + * dvbpsi_network_name_dr_t
> + *****************************************************************************/
> +/*!
> + * \struct dvbpsi_network_name_dr_t
> + * \brief "network name" descriptor structure.
> + *
> + * This structure is used to store a decoded "network name"
> + * descriptor. (ETSI EN 300 468 section 6.2.27).
> + */
> +/*!
> + * \typedef struct dvbpsi_network_nameg_dr_s dvbpsi_network_name_dr_t
> + * \brief dvbpsi_network_name_dr_t type definition.
> + */
> +typedef struct dvbpsi_network_name_dr_s
> +{
> + uint8_t i_name_length; /*!< length of the i_name_byte
> + array */
> + uint8_t i_name_byte[255]; /*!< the name of the delivery system */
Is the name of the delivery system always < 256 characters?
i_name_length could be just i_length (<=== my preference).
i_name_byte[] is the wrong variable name, it should be p_byte or
p_name (although it is technically not a pointer here, but refers
array of bytes).
> +
> +} dvbpsi_network_name_dr_t;
> +
> +/*****************************************************************************
> + * dvbpsi_DecodeNetworkNameDr
> + *****************************************************************************/
> +/*!
> + * \fn dvbpsi_network_name_dr_t * dvbpsi_DecodeNetworkNameDr(
> + dvbpsi_descriptor_t * p_descriptor)
> + * \brief "network name" descriptor decoder.
> + * \param p_descriptor pointer to the descriptor structure
> + * \return a pointer to a new "network name" descriptor structure
> + * which contains the decoded data.
> + */
> +dvbpsi_network_name_dr_t* dvbpsi_DecodeNetworkNameDr(
> + dvbpsi_descriptor_t * p_descriptor);
> +
> +
> +/*****************************************************************************
> + * dvbpsi_GenNetworkNameDr
> + *****************************************************************************/
> +/*!
> + * \fn dvbpsi_descriptor_t * dvbpsi_GenNetworkNameDr(
> + dvbpsi_network_name_dr_t * p_decoded, int b_duplicate)
> + * \brief "network name" descriptor generator.
> + * \param p_decoded pointer to a decoded "network name" 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_GenNetworkNameDr(
> + dvbpsi_network_name_dr_t * p_decoded,
> + int b_duplicate);
'int b_duplicate' should be a bool now, thus 'bool b_duplicate'.
> +
> +#ifdef __cplusplus
> +};
> +#endif
drop the #ifdef __cplusplus .. #endif defines
It is not done in the entire library, so starting here is of little
use for that.
> +
> +#else
> +#error "Multiple inclusions of dr_40.h"
> +#endif
> --
> 1.7.0.4
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> http://mailman.videolan.org/listinfo/vlc-devel
Kind regards,
Jean-Paul Saman
More information about the libdvbpsi-devel
mailing list