[vlc-commits] [Git][videolan/vlc][master] 11 commits: Revert "demux:mkv: fix wrong value reset after clean of an array"

Steve Lhomme (@robUx4) gitlab at videolan.org
Sun Oct 19 16:21:55 UTC 2025



Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
f9500c23 by Steve Lhomme at 2025-10-19T15:30:42+00:00
Revert "demux:mkv: fix wrong value reset after clean of an array"

This reverts commit d1d41b3f849d4e260c1ec09c5b1bb53db7f8107f.

The i_subpackets value represents the size of the p_subpackets array.
It should not change during the lifetime of that array.

The i_subpacket value is the count of actually filled elements in the array.
When the array is fully loaded we can send the blocks for decoding.

The fix for the hacker one file should be different.

- - - - -
b01b9442 by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: only send actual blocks

If i_sub_packet_h/i_sub_packet_h/i_subpacket_size have odd values,
the index may of filled blocks may not cover all the arrays.

We send the blocks that are at the proper place in hope the decoder
can make something of it.

Fixes https://hackerone.com/reports/491495

- - - - -
80fae6db by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: pass VLC_CODEC_xxx as vlc_fourcc_t

- - - - -
7af48e29 by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: check the real_audio_private size based on packed size

- - - - -
b714e22d by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: read the cook/atrac3 blockalign in A_REAL__helper

So that the real_audio_private handling is handled in one location.

- - - - -
547448a0 by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: add a simple byte reader class

Faster than bs_t for bytes and can handle little/big-endian.

- - - - -
7cac9cbe by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: parse RealAudio extradata directly in Cook_PrivateTrackData

Using a byte reader to avoid relying on unpacked structures.

Fixes #29420

- - - - -
04f5babb by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: move the RealAudio structures in util.cpp

- - - - -
216e30ef by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: use the coded_frame_size for RA288 block alignment

As done in libavformat.

- - - - -
49eaa39a by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: use a C++ vector for the RealAudio sub packets

This is less error-prone.

- - - - -
c9af0b78 by Steve Lhomme at 2025-10-19T15:30:42+00:00
demux: mkv: use safer cast to Cook_PrivateTrackData

- - - - -


3 changed files:

- modules/demux/mkv/matroska_segment_parse.cpp
- modules/demux/mkv/util.cpp
- modules/demux/mkv/util.hpp


Changes:

=====================================
modules/demux/mkv/matroska_segment_parse.cpp
=====================================
@@ -2107,7 +2107,7 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
                 memset( &p_extra[18], 0, 30  - 18 );
             }
         }
-        static void A_PCM__helper (HandlerPayload& vars, uint32_t i_codec) {
+        static void A_PCM__helper (HandlerPayload& vars, vlc_fourcc_t i_codec) {
             ONLY_FMT(AUDIO);
             vars.p_fmt->i_codec = i_codec;
             vars.p_fmt->audio.i_blockalign = ( vars.p_fmt->audio.i_bitspersample + 7 ) / 8 * vars.p_fmt->audio.i_channels;
@@ -2127,7 +2127,7 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
             ONLY_FMT(AUDIO);
             uint8_t *p = vars.p_tk->p_extra_data;
 
-            if (vars.p_tk->i_extra_data <= sizeof(real_audio_private))
+            if (vars.p_tk->i_extra_data <= SIZEOF_REALAUDIO_PRIVATE)
                 return false;
 
             if( memcmp( p, ".ra", 3 ) ) {
@@ -2138,41 +2138,31 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
 
             return true;
         }
-        static void A_REAL__helper (HandlerPayload& vars, uint32_t i_codec) {
+        static void A_REAL__helper (HandlerPayload& vars, vlc_fourcc_t i_codec) {
             mkv_track_t        * p_tk = vars.p_tk;
-            real_audio_private * priv = (real_audio_private*) p_tk->p_extra_data;
 
             p_tk->fmt.i_codec = i_codec;
 
-            /* FIXME RALF and SIPR */
-            uint16_t version = (uint16_t) hton16(priv->version);
-
-            p_tk->p_sys = new Cook_PrivateTrackData(
-                  hton16( priv->sub_packet_h ),
-                  hton16( priv->frame_size ),
-                  hton16( priv->sub_packet_size )
+            Cook_PrivateTrackData * p_realaudio = new Cook_PrivateTrackData(
+                p_tk->p_extra_data,
+                p_tk->i_extra_data
             );
+            p_tk->p_sys = p_realaudio;
 
-            if( unlikely( !p_tk->p_sys ) )
-                throw std::runtime_error ("p_tk->p_sys is NULL when handling A_REAL/28_8");
+            if( unlikely( !p_realaudio ) )
+                throw std::runtime_error ("Cook_PrivateTrackData is NULL when handling A_REAL/28_8");
 
-            if( unlikely( p_tk->p_sys->Init() ) )
-                throw std::runtime_error ("p_tk->p_sys->Init() failed when handling A_REAL/28_8");
+            if( unlikely( p_realaudio->Init() ) )
+                throw std::runtime_error ("Cook_PrivateTrackData::Init() failed when handling A_REAL/28_8");
 
-            if( version == 4 )
-            {
-                real_audio_private_v4 * v4 = (real_audio_private_v4*) priv;
-                p_tk->fmt.audio.i_channels = hton16(v4->channels);
-                p_tk->fmt.audio.i_bitspersample = hton16(v4->sample_size);
-                p_tk->fmt.audio.i_rate = hton16(v4->sample_rate);
-            }
-            else if( version == 5 )
-            {
-                real_audio_private_v5 * v5 = (real_audio_private_v5*) priv;
-                p_tk->fmt.audio.i_channels = hton16(v5->channels);
-                p_tk->fmt.audio.i_bitspersample = hton16(v5->sample_size);
-                p_tk->fmt.audio.i_rate = hton16(v5->sample_rate);
-            }
+            if (i_codec == VLC_CODEC_COOK || i_codec == VLC_CODEC_ATRAC3)
+                p_tk->fmt.audio.i_blockalign = p_realaudio->i_subpacket_size;
+            else // VLC_CODEC_RA_288
+                p_tk->fmt.audio.i_blockalign = p_realaudio->coded_frame_size;
+
+            p_tk->fmt.audio.i_rate          = p_realaudio->i_rate;
+            p_tk->fmt.audio.i_bitspersample = p_realaudio->i_bitspersample;
+            p_tk->fmt.audio.i_channels      = p_realaudio->i_channels;
             msg_Dbg(vars.p_demuxer, "%d channels %d bits %d Hz",p_tk->fmt.audio.i_channels, p_tk->fmt.audio.i_bitspersample, p_tk->fmt.audio.i_rate);
 
             fill_extra_data( p_tk, p_tk->fmt.i_codec == VLC_CODEC_RA_288 ? 0 : 78);
@@ -2181,18 +2171,12 @@ bool matroska_segment_c::TrackInit( mkv_track_t * p_tk )
             if (!A_REAL__is_valid (vars))
                 return;
 
-            real_audio_private * priv = (real_audio_private*) vars.p_tk->p_extra_data;
-            vars.p_tk->fmt.audio.i_blockalign = hton16(priv->sub_packet_size);
-
             A_REAL__helper (vars, VLC_CODEC_COOK);
         }
         S_CASE("A_REAL/ATRC") {
             if (!A_REAL__is_valid (vars))
                 return;
 
-            real_audio_private * priv = (real_audio_private*) vars.p_tk->p_extra_data;
-            vars.p_tk->fmt.audio.i_blockalign = hton16(priv->sub_packet_size);
-
             A_REAL__helper (vars, VLC_CODEC_ATRAC3);
         }
         S_CASE("A_REAL/28_8") {


=====================================
modules/demux/mkv/util.cpp
=====================================
@@ -169,19 +169,18 @@ block_t *MemToBlock( uint8_t *p_mem, size_t i_mem, size_t offset)
 void handle_real_audio(demux_t * p_demux, mkv_track_t * p_tk, block_t * p_blk, vlc_tick_t i_pts)
 {
     uint8_t * p_frame = p_blk->p_buffer;
-    Cook_PrivateTrackData * p_sys = (Cook_PrivateTrackData *) p_tk->p_sys;
+    Cook_PrivateTrackData * p_sys = static_cast<Cook_PrivateTrackData *>(p_tk->p_sys);
     size_t size = p_blk->i_buffer;
 
     if( p_tk->i_last_dts == VLC_TICK_INVALID )
     {
-        for( size_t i = 0; i < p_sys->i_subpackets; i++)
+        for( size_t i = 0; i < p_sys->p_subpackets.size(); i++)
             if( p_sys->p_subpackets[i] )
             {
                 block_Release(p_sys->p_subpackets[i]);
                 p_sys->p_subpackets[i] = NULL;
             }
         p_sys->i_subpacket = 0;
-        p_sys->i_subpackets = 0;
 
         if ( !( p_blk->i_flags & BLOCK_FLAG_TYPE_I) )
         {
@@ -203,45 +202,51 @@ void handle_real_audio(demux_t * p_demux, mkv_track_t * p_tk, block_t * p_blk, v
         {
             size_t i_index = (size_t) p_sys->i_sub_packet_h * i +
                           ((p_sys->i_sub_packet_h + 1) / 2) * (y&1) + (y>>1);
-            if( i_index >= p_sys->i_subpackets )
+            if( unlikely(i_index >= p_sys->p_subpackets.size()) )
                 return;
 
             if( size < p_sys->i_subpacket_size )
                 return;
 
-            block_t *p_block = block_Alloc( p_sys->i_subpacket_size );
-            if( !p_block )
-                return;
-
-            memcpy( p_block->p_buffer, p_frame, p_sys->i_subpacket_size );
-            p_block->i_dts = VLC_TICK_INVALID;
-            p_block->i_pts = VLC_TICK_INVALID;
-            if( !p_sys->i_subpacket )
+            if (likely(p_sys->p_subpackets[i_index] == nullptr))
             {
-                p_tk->i_last_dts =
-                p_block->i_pts = i_pts;
+                // the index was not used
+                block_t *p_block = block_Alloc( p_sys->i_subpacket_size );
+                if( !p_block )
+                    return;
+
+                memcpy( p_block->p_buffer, p_frame, p_sys->i_subpacket_size );
+                p_block->i_dts = VLC_TICK_INVALID;
+                p_block->i_pts = VLC_TICK_INVALID;
+                if( p_sys->i_subpacket == 0 )
+                {
+                    p_tk->i_last_dts =
+                    p_block->i_pts = i_pts;
+                }
+                p_sys->p_subpackets[i_index] = p_block;
             }
 
             p_frame += p_sys->i_subpacket_size;
             size -=  p_sys->i_subpacket_size;
 
             p_sys->i_subpacket++;
-            p_sys->p_subpackets[i_index] = p_block;
         }
     }
     else
     {
         /*TODO*/
     }
-    if( p_sys->i_subpacket == p_sys->i_subpackets )
+    if( p_sys->i_subpacket == p_sys->p_subpackets.size() )
     {
-        for( size_t i = 0; i < p_sys->i_subpackets; i++)
+        for( size_t i = 0; i < p_sys->p_subpackets.size(); i++)
         {
-            send_Block( p_demux, p_tk, p_sys->p_subpackets[i], 1, 0 );
-            p_sys->p_subpackets[i] = NULL;
+            if (likely(p_sys->p_subpackets[i]))
+            {
+                send_Block( p_demux, p_tk, p_sys->p_subpackets[i], 1, 0 );
+                p_sys->p_subpackets[i] = NULL;
+            }
         }
         p_sys->i_subpacket = 0;
-        p_sys->i_subpackets = 0;
     }
 }
 
@@ -385,27 +390,95 @@ void send_Block( demux_t * p_demux, mkv_track_t * p_tk, block_t * p_block, unsig
     es_out_Send( p_demux->out, p_tk->p_es, p_block);
 }
 
-int32_t Cook_PrivateTrackData::Init()
+struct real_audio_private
+{
+    uint32_t fourcc;
+    uint16_t version;
+    uint16_t unknown1;
+    uint8_t  unknown2[12];
+    uint16_t unknown3;
+    uint16_t flavor;
+    uint32_t coded_frame_size;
+    uint32_t unknown4[3];
+    uint16_t sub_packet_h;
+    uint16_t frame_size;
+    uint16_t sub_packet_size;
+    uint16_t unknown5;
+};
+
+struct real_audio_private_v4
 {
-    i_subpackets = (size_t) i_sub_packet_h * (size_t) i_frame_size / (size_t) i_subpacket_size;
-    p_subpackets = static_cast<block_t**> ( calloc(i_subpackets, sizeof(block_t*)) );
+    real_audio_private header;
+    uint16_t sample_rate;
+    uint16_t unknown;
+    uint16_t sample_size;
+    uint16_t channels;
+};
 
-    if( unlikely( !p_subpackets ) )
+
+struct real_audio_private_v5
+{
+    real_audio_private header;
+    uint32_t unknown1;
+    uint16_t unknown2;
+    uint16_t sample_rate;
+    uint16_t unknown3;
+    uint16_t sample_size;
+    uint16_t channels;
+};
+
+int32_t Cook_PrivateTrackData::Init()
+{
+    // real_audio_private
+    bytes.skip(4);                      // fourcc
+
+    /* FIXME RALF and SIPR */
+    uint16_t version = bytes.GetBE16(); // version
+    bytes.skip(2);                      // unknown1
+    bytes.skip(12);                     // unknown2
+    bytes.skip(2);                      // unknown3
+    bytes.skip(2);                      // flavor
+    coded_frame_size = bytes.GetBE32(); // coded_frame_size
+    bytes.skip(3*4);                    // unknown4
+    i_sub_packet_h   = bytes.GetBE16(); // sub_packet_h
+    i_frame_size     = bytes.GetBE16(); // frame_size
+    i_subpacket_size = bytes.GetBE16(); // sub_packet_size
+    bytes.skip(2);                      // unknown5
+
+    if( version == 4 )
     {
-        i_subpackets = 0;
-        return 1;
+        // real_audio_private_v4
+        i_rate          = bytes.GetBE16(); // sample_rate
+        bytes.skip(2);                     // unknown
+        i_bitspersample = bytes.GetBE16(); // sample_size
+        i_channels      = bytes.GetBE16(); // channels
     }
+    else if( version == 5 )
+    {
+        // real_audio_private_v5
+        bytes.skip(4);                     // unknown1
+        bytes.skip(2);                     // unknown2
+        i_rate          = bytes.GetBE16(); // sample_rate
+        bytes.skip(2);                     // unknown2
+        i_bitspersample = bytes.GetBE16(); // sample_size
+        i_channels      = bytes.GetBE16(); // channels
+    }
+    else
+        return 0;
+    if (bytes.hasErrors())
+        return 0;
+
+    size_t i_subpackets = (size_t) i_sub_packet_h * (size_t) i_frame_size / (size_t) i_subpacket_size;
+    p_subpackets.resize(i_subpackets);
 
     return 0;
 }
 
 Cook_PrivateTrackData::~Cook_PrivateTrackData()
 {
-    for( size_t i = 0; i < i_subpackets; i++ )
+    for( size_t i = 0; i < p_subpackets.size(); i++ )
         if( p_subpackets[i] )
             block_Release( p_subpackets[i] );
-
-    free( p_subpackets );
 }
 
 static inline void fill_wvpk_block(uint16_t version, uint32_t block_samples, uint32_t flags,


=====================================
modules/demux/mkv/util.hpp
=====================================
@@ -38,59 +38,81 @@ block_t *WEBVTT_Repack_Sample(block_t *p_block, bool b_webm = false,
 void send_Block( demux_t * p_demux, mkv_track_t * p_tk, block_t * p_block, unsigned int i_number_frames, int64_t i_duration );
 int UpdatePCR( demux_t * p_demux );
 
-
-struct real_audio_private
+class ByteReader
 {
-    uint32_t fourcc;
-    uint16_t version;
-    uint16_t unknown1;
-    uint8_t  unknown2[12];
-    uint16_t unknown3;
-    uint16_t flavor;
-    uint32_t coded_frame_size;
-    uint32_t unknown4[3];
-    uint16_t sub_packet_h;
-    uint16_t frame_size;
-    uint16_t sub_packet_size;
-    uint16_t unknown5;
-};
+public:
+    ByteReader(const uint8_t *reader_, size_t reader_len_):
+        reader(reader_), reader_left(reader_len_) {}
 
-struct real_audio_private_v4
-{
-    real_audio_private header;
-    uint16_t sample_rate;
-    uint16_t unknown;
-    uint16_t sample_size;
-    uint16_t channels;
-};
+    bool skip(size_t bytes) {
+        if (error) return false;
+        if (reader_left < bytes) {
+            reader_left = 0;
+            error = true;
+            return false;
+        }
+        reader_left -= bytes;
+        reader += bytes;
+        return true;
+    }
 
+    uint16_t GetBE16() {
+        if (error) return 0;
+        if (reader_left < 2) {
+            reader_left = 0;
+            error = true;
+            return 0;
+        }
+        uint16_t v = GetWBE(reader);
+        reader_left -= 2;
+        reader += 2;
+        return v;
+    }
 
-struct real_audio_private_v5
-{
-    real_audio_private header;
-    uint32_t unknown1;
-    uint16_t unknown2;
-    uint16_t sample_rate;
-    uint16_t unknown3;
-    uint16_t sample_size;
-    uint16_t channels;
+    uint32_t GetBE32() {
+        if (error) return 0;
+        if (reader_left < 4) {
+            reader_left = 0;
+            error = true;
+            return 0;
+        }
+        uint16_t v = GetDWBE(reader);
+        reader_left -= 4;
+        reader += 4;
+        return v;
+    }
+
+    bool hasErrors() const { return error; }
+
+private:
+    const uint8_t *reader;
+    size_t        reader_left;
+    bool          error = false;
 };
 
+#define SIZEOF_REALAUDIO_PRIVATE  (4+2+2+12+2+2+4+(3*4)+2+2+2+2)
+
 class Cook_PrivateTrackData : public PrivateTrackData
 {
 public:
-    Cook_PrivateTrackData(uint16_t sph, uint16_t fs, uint16_t sps):
-        i_sub_packet_h(sph), i_frame_size(fs), i_subpacket_size(sps),
-        p_subpackets(NULL), i_subpackets(0), i_subpacket(0){}
+    Cook_PrivateTrackData(const uint8_t *reader, size_t reader_len):
+        bytes(reader, reader_len) {}
     ~Cook_PrivateTrackData();
     int32_t Init();
 
+    uint32_t coded_frame_size;
     uint16_t i_sub_packet_h;
     uint16_t i_frame_size;
     uint16_t i_subpacket_size;
-    block_t  **p_subpackets;
-    size_t   i_subpackets;
-    size_t   i_subpacket;
+
+    uint16_t i_rate;
+    uint16_t i_bitspersample;
+    uint16_t i_channels;
+
+    std::vector<block_t *> p_subpackets;
+    size_t   i_subpacket = 0;
+private:
+    ByteReader bytes;
 };
 
 block_t * packetize_wavpack( const mkv_track_t &, uint8_t *, size_t);



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/43da4df125d484066576ffcfecd9ba1a8b15c3ff...c9af0b78afddc0ce13e759335e3af1431be08719

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/43da4df125d484066576ffcfecd9ba1a8b15c3ff...c9af0b78afddc0ce13e759335e3af1431be08719
You're receiving this email because of your account on code.videolan.org.


VideoLAN code repository instance


More information about the vlc-commits mailing list