[vlc-devel] [PATCH] wav: add RF64 support
Tobias Rapp
t.rapp at noa-audio.com
Thu May 2 10:26:09 CEST 2013
Rémi Denis-Courmont wrote:
> Le mardi 30 avril 2013 16:54:07, Tobias Rapp a écrit :
> > Add support for wave files > 4GB. See also the EBU specification at
> > http://tech.ebu.ch/docs/tech/tech3306-2009.pdf
> >
> > ---
> > include/vlc_codecs.h | 12 ++++++++++++
> > modules/demux/wav.c | 48 ++++++++++++++++++++++++++++++++++++++++++++
> > +--- 2 files changed, 57 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/vlc_codecs.h b/include/vlc_codecs.h
> > index f2b8daf..db09924 100644
> > --- a/include/vlc_codecs.h
> > +++ b/include/vlc_codecs.h
> > @@ -109,6 +109,18 @@ _WAVEHEADER {
> > } WAVEHEADER;
> > #endif /* _WAVEHEADER_ */
> >
> > +#ifndef _WAVEDATASIZE64_
> > +#define _WAVEDATASIZE64_
> > +typedef struct
> > +ATTR_PACKED
> > +_WAVEDATASIZE64 {
> > + uint64_t RiffSize;
> > + uint64_t DataSize;
> > + uint64_t SampleCount;
> > + uint32_t TableLength;
> > +} WAVEDATASIZE64;
> > +#endif /* _WAVEDATASIZE64_ */
> > +
>
> Errm, is this actually defined by Windows? If not, I don't see the point in
> pretending it is. Also note that packed is not portable.
No, it is not. Have updated the patch to work without a packed struct, see
attached file.
> > #ifndef _VLC_BITMAPINFOHEADER_
> > #define _VLC_BITMAPINFOHEADER_
> >
> > diff --git a/modules/demux/wav.c b/modules/demux/wav.c
> > index 6aefc92..de7e740 100644
> > --- a/modules/demux/wav.c
> > +++ b/modules/demux/wav.c
> > @@ -61,7 +61,7 @@ struct demux_sys_t
> > es_out_id_t *p_es;
> >
> > int64_t i_data_pos;
> > - unsigned int i_data_size;
> > + int64_t i_data_size;
>
> Why is this signed?
Because it is often combined with i_data_pos and compared to the result of
stream_Tell, both of them are signed int64. Have added an overflow check
similar to how it is done in stream_Tell in the updated patch.
> >
> > unsigned int i_frame_size;
> > int i_frame_samples;
> > @@ -100,18 +100,22 @@ static int Open( vlc_object_t * p_this )
> > demux_sys_t *p_sys;
> >
> > const uint8_t *p_peek;
> > + bool b_is_rf64;
> > unsigned int i_size;
> > unsigned int i_extended;
> > const char *psz_name;
> >
> > WAVEFORMATEXTENSIBLE *p_wf_ext = NULL;
> > WAVEFORMATEX *p_wf = NULL;
> > + WAVEDATASIZE64 *p_wds = NULL;
> >
> > /* Is it a wav file ? */
> > if( stream_Peek( p_demux->s, &p_peek, 12 ) < 12 )
> > return VLC_EGENERIC;
> >
> > - if( memcmp( p_peek, "RIFF", 4 ) || memcmp( &p_peek[8], "WAVE", 4 ) )
> > + b_is_rf64 = ( memcmp( p_peek, "RF64", 4 ) == 0 );
> > + if( ( !b_is_rf64 && memcmp( p_peek, "RIFF", 4 ) ) ||
> > + memcmp( &p_peek[8], "WAVE", 4 ) )
> > {
> > return VLC_EGENERIC;
> > }
> > @@ -123,6 +127,7 @@ static int Open( vlc_object_t * p_this )
> > return VLC_ENOMEM;
> >
> > p_sys->p_es = NULL;
> > + p_sys->i_data_size = 0;
> > p_sys->i_chans_to_reorder = 0;
> > p_sys->i_channel_mask = 0;
> >
> > @@ -130,6 +135,41 @@ static int Open( vlc_object_t * p_this )
> > if( stream_Read( p_demux->s, NULL, 12 ) != 12 )
> > goto error;
> >
> > + if( b_is_rf64 )
> > + {
> > + /* search datasize64 chunk */
> > + if( ChunkFind( p_demux, "ds64", &i_size ) )
> > + {
> > + msg_Err( p_demux, "cannot find 'ds64' chunk" );
> > + goto error;
> > + }
> > + i_size += 2;
>
> Integer overflow.
Fixed.
Note that a similar code pattern is used for reading the wave format chunk.
Will send a separate patch for that later.
> > + if( i_size < sizeof( WAVEDATASIZE64 ) )
> > + {
> > + msg_Err( p_demux, "invalid 'ds64' chunk" );
> > + goto error;
> > + }
> > + if( stream_Read( p_demux->s, NULL, 8 ) != 8 )
> > + goto error;
> > +
> > + /* load datasize64 */
> > + p_wds = malloc( i_size );
> > + if( unlikely( !p_wds ) )
> > + goto error;
> > +
> > + i_size -= 2;
> > + if( stream_Read( p_demux->s, p_wds, i_size ) != (int)i_size ||
> > + ( ( i_size & 1 ) && stream_Read( p_demux->s, NULL, 1 ) != 1 )
> > ) + {
> > + msg_Err( p_demux, "cannot load 'ds64' chunk" );
> > + free( p_wds );
> > + goto error;
> > + }
> > + p_sys->i_data_size = GetQWLE(&p_wds->DataSize);
> > +
> > + free( p_wds );
> > + }
> > +
> > /* search fmt chunk */
> > if( ChunkFind( p_demux, "fmt ", &i_size ) )
> > {
> > @@ -365,11 +405,13 @@ static int Open( vlc_object_t * p_this )
> >
> > msg_Dbg( p_demux, "found %s audio format", psz_name );
> >
> > - if( ChunkFind( p_demux, "data", &p_sys->i_data_size ) )
> > + if( ChunkFind( p_demux, "data", &i_size ) )
> > {
> > msg_Err( p_demux, "cannot find 'data' chunk" );
> > goto error;
> > }
> > + if( !b_is_rf64 || i_size < UINT32_MAX )
>
> Isn't this a tautology?
Not really. The RF64 spec states:
If the 32-bit value in the [chunk size] field is not "-1" (= FFFFFFFF hex)
then this 32-bit value is used. If the 32-bit value in the field is "-1" the
64-bit value in the 'ds64' chunk is used instead.
> > + p_sys->i_data_size = i_size;
> > if( stream_Read( p_demux->s, NULL, 8 ) != 8 )
> > goto error;
> > p_sys->i_data_pos = stream_Tell( p_demux->s );
>
Thanks for the review,
Tobias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-wav-add-RF64-support.patch
Type: text/x-patch
Size: 3393 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20130502/a410eaa6/attachment.bin>
More information about the vlc-devel
mailing list