[vlc-devel] [PATCH] vobsub idx in mkv track CodecPrivate data

[Laurent Aimar]

Hi,

On Tue, May 12, 2009, John Stebbins wrote:
> diff --git a/modules/demux/avformat/demux.c b/modules/demux/avformat/demux.c
> index dd0f7c9..9224c62 100644
> --- a/modules/demux/avformat/demux.c
> +++ b/modules/demux/avformat/demux.c
> @@ -47,6 +47,7 @@
>  
>  #include "../../codec/avcodec/avcodec.h"
>  #include "avformat.h"
> +#include "../vobsub.h"
>  
>  //#define AVFORMAT_DEBUG 1
>  
> @@ -271,6 +272,48 @@ int OpenDemux( vlc_object_t *p_this )
>  
>          case CODEC_TYPE_SUBTITLE:
>              es_format_Init( &fmt, SPU_ES, fcc );
> +            if( strncmp( p_sys->ic->iformat->name, "matroska", 8 ) == 0 &&
> +                cc->codec_id == CODEC_ID_DVD_SUBTITLE &&
> +                cc->extradata != NULL &&
> +                cc->extradata_size > 0 )
> +            {
> +                char *p_start;
> +                char *p_buf = malloc( cc->extradata_size + 1);

 To match code style of the file, I would prefer psz_ prefix

> diff --git a/modules/demux/mkv/matroska_segment.cpp b/modules/demux/mkv/matroska_segment.cpp
> index 26d43b4..0797b91 100644
> --- a/modules/demux/mkv/matroska_segment.cpp
> +++ b/modules/demux/mkv/matroska_segment.cpp
> @@ -28,6 +28,10 @@
>  
>  #include "demux.hpp"
>  
> +extern "C" {
> +#include "../vobsub.h"
> +}
> +
>  /* GetFourCC helper */
>  #define GetFOURCC( p )  __GetFOURCC( (uint8_t*)p )
>  static vlc_fourcc_t __GetFOURCC( uint8_t *p )
> @@ -1097,20 +1101,39 @@ bool matroska_segment_c::Select( mtime_t i_start_time )
>              {
>                  char *p_start;
>                  char *p_buf = (char *)malloc( tracks[i_track]->i_extra_data + 1);
 Same here.

> +                if (p_buf != NULL)
if( 

> +static inline bool vobsub_palette_parse( const char *buf, uint32_t *palette )
 In VLC, we usually use int with VLC_EGENERIC/VLC_SUCCESS to return
error status (unless the name of your function makes it obvious it is a bool).

> +{
> +    if( sscanf( buf, "palette: %x, %x, %x, %x, %x, %x, %x, %x, "
> +                              "%x, %x, %x, %x, %x, %x, %x, %x",
> +                &palette[0], &palette[1], &palette[2], &palette[3],
> +                &palette[4], &palette[5], &palette[6], &palette[7],
> +                &palette[8], &palette[9], &palette[10], &palette[11],
> +                &palette[12], &palette[13], &palette[14], &palette[15] ) == 16 )
 %x cannot be used for uint32_t, you need:
 - to use temporary unsigned variables
 - or to use PRIx32 like "palette: %"PRIx32", %"PRIx32", " [...]
(unsigned is not the same than uint32_t)

> +static inline bool vobsub_size_parse( const char *buf, int *original_frame_width, int *original_frame_height )
 I would prefer using the prefix psz_ and pi_ but as it's a new file you
don't have too as long as it is consistant.

 Could you send it using git format-patch ? It would be better but if you
don't/can't I will workaround it.

Regards,

-- 
fenrir