[vlc-devel] [PATCH v3] Add support for GoPro HiLight tags as chapters

[Filip Roséen]

On 16/05/24 20:42, Emeric Grange wrote:

> +/* GoPro HiLight tags support */
> +static int MP4_ReadBox_HMMT( stream_t *p_stream, MP4_Box_t *p_box )
> +{
> +    MP4_Box_data_HMMT_t *p_hmmt;
> +    MP4_READBOX_ENTER( MP4_Box_data_HMMT_t, NULL );
> +
> +    if ( i_read < 5 )
> +        MP4_READBOX_EXIT( 0 );
> +
> +    p_hmmt = p_box->data.p_hmmt;
> +
> +    MP4_GET4BYTES( p_hmmt->i_chapter_count );
> +
> +    if ( i_read < p_hmmt->i_chapter_count*4 )
> +        MP4_READBOX_EXIT( 0 );

I am not truely familiar with the `MP4_READBOX_ENTER` macro, but after skimming
through its implementation I discovered an issue with the above implementation.

See [this testcase](http://codepad.org/gTFDYk8G), or the attached file, for a
simulation of the above. Given that the expression `p_hmmt->i_chapter_count*4`
might result in a value that will overflow, the if-statement itself is faulty.

Changing the code to something like the below is a suitable workaround (though
it could certainly use a comment or a constant instead of the magic `4`):

    if( ( i_read / 4 ) < p_hmmt->i_chapter_count )
        MP4_READBOX_EXIT( 0 );


Disclaimer
----------

 - *I am currently insanely tired and just skimmed the patch in this thread
   while having a smoke, I in no way claim that I am fit for fight for reviews
   at this hour.*

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160525/1c454241/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: testcase.c
Type: text/x-c
Size: 536 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20160525/1c454241/attachment.bin>