[vlc-devel] [RFC] demux/xa: Open: fix xa-header parsing

[Filip Roséen]

If a compiler introduce padding between the members of struct
xa_header, the previous way of interpreting the incoming stream will
yield unexpected behavior.

These changes make sure that we interpret the data in a way that is
uniform across platforms; not relying on implementation-defined
behavior.

--

This patch is untested and is therefor marked as RFC; the only thing that is
different, other than defined parsing, is that the previous implementation did
not to any byte-swapping (to respect endianess) for p_xa.iSize before using it.

Other than that I tried keeping the changes to a minimum, even though there are
pending clean-up patches that I will submit if this one gets merged.

Thanks.

---
 modules/demux/xa.c | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/modules/demux/xa.c b/modules/demux/xa.c
index 028e3568ed..cb48113799 100644
--- a/modules/demux/xa.c
+++ b/modules/demux/xa.c
@@ -79,6 +79,7 @@ typedef struct xa_header_t
     uint16_t wBitsPerSample;
 } xa_header_t;
 
+#define HEADER_LENGTH ( 4 + 4 + 2 + 2 + 4 + 4 + 2 + 2 )
 #define FRAME_LENGTH 28 /* samples per frame */
 
 /*****************************************************************************
@@ -88,19 +89,30 @@ static int Open( vlc_object_t * p_this )
 {
     demux_t     *p_demux = (demux_t*)p_this;
     demux_sys_t *p_sys;
-    xa_header_t p_xa;
+    xa_header_t header;
     const uint8_t *p_buf;
 
     /* XA file heuristic */
-    if( vlc_stream_Peek( p_demux->s, &p_buf, sizeof( p_xa ) )
-            < (signed)sizeof( p_xa ) )
+    if( vlc_stream_Peek( p_demux->s, &p_buf, HEADER_LENGTH ) < HEADER_LENGTH )
         return VLC_EGENERIC;
 
-    memcpy( &p_xa, p_buf, sizeof( p_xa ) );
-    if( ( strncmp( p_xa.xa_id, "XAI", 4 )
-       && strncmp( p_xa.xa_id, "XAJ", 4 ) )
-     || ( GetWLE( &p_xa.wFormatTag  ) != 0x0001)
-     || ( GetWLE( &p_xa.wBitsPerSample ) != 16) )
+    memcpy( &header.xa_id, p_buf, sizeof( header.xa_id ) );
+
+    header.iSize = GetDWLE( p_buf + 4 );
+    header.wFormatTag = GetWLE( p_buf + 8 );
+    header.nChannels = GetWLE( p_buf + 10 );
+    header.nSamplesPerSec = GetDWLE( p_buf + 12 );
+    header.nAvgBytesPerSec = GetDWLE( p_buf + 16 );
+    header.nBlockAlign = GetWLE( p_buf + 20 );
+    header.wBitsPerSample = GetWLE( p_buf + 22 );
+
+    if( strncmp( header.xa_id, "XAI", 4 ) && strncmp( header.xa_id, "XAJ", 4 ) )
+        return VLC_EGENERIC;
+
+    if( header.wFormatTag != 0x0001 )
+        return VLC_EGENERIC;
+
+    if( header.wBitsPerSample != 16 )
         return VLC_EGENERIC;
 
     p_sys = malloc( sizeof( demux_sys_t ) );
@@ -112,18 +124,21 @@ static int Open( vlc_object_t * p_this )
     p_demux->p_sys      = p_sys;
     p_sys->p_es         = NULL;
 
-    /* skip XA header -- cannot fail */
-    vlc_stream_Read( p_demux->s, NULL, sizeof( p_xa ) );
+    if( vlc_stream_Read( p_demux->s, NULL, HEADER_LENGTH ) != HEADER_LENGTH )
+    {
+        free( p_sys );
+        return VLC_EGENERIC;
+    }
 
     es_format_t fmt;
     es_format_Init( &fmt, AUDIO_ES, VLC_FOURCC('X','A','J',0) );
 
     msg_Dbg( p_demux, "assuming EA ADPCM audio codec" );
-    fmt.audio.i_rate = GetDWLE( &p_xa.nSamplesPerSec );
-    fmt.audio.i_bytes_per_frame = 15 * GetWLE( &p_xa.nChannels );
+    fmt.audio.i_rate = header.nSamplesPerSec;
+    fmt.audio.i_bytes_per_frame = 15 * header.nChannels;
     fmt.audio.i_frame_length = FRAME_LENGTH;
 
-    fmt.audio.i_channels = GetWLE ( &p_xa.nChannels );
+    fmt.audio.i_channels = header.nChannels;
     fmt.audio.i_blockalign = fmt.audio.i_bytes_per_frame;
     fmt.audio.i_bitspersample = 16;
     fmt.i_bitrate = (fmt.audio.i_rate * fmt.audio.i_bytes_per_frame * 8)
@@ -131,7 +146,7 @@ static int Open( vlc_object_t * p_this )
 
     p_sys->i_data_offset = vlc_stream_Tell( p_demux->s );
     /* FIXME: better computation */
-    p_sys->i_data_size = p_xa.iSize * 15 / 56;
+    p_sys->i_data_size = header.iSize * 15 / 56;
     /* How many frames per block (1:1 is too CPU intensive) */
     p_sys->i_block_frames = fmt.audio.i_rate / (FRAME_LENGTH * 20) + 1;
     p_sys->i_frame_size = fmt.audio.i_bytes_per_frame;
-- 
2.11.1