[vlc-devel] [PATCH 02/11] demux: wav: refactor chunk skip

[Thomas Guillem]

    if( unlikely( i_size >= 65536 ) )
    {
        /* Arbitrary size where a seek should be performed instead of skipping
         * by reading NULL. Non data chunks are generally smaller than this.
         * This seek may be used to skip the data chunk if there is an other
         * chunk after it (unlikely). */
        return vlc_stream_Seek( p_demux->s,
                                vlc_stream_Tell( p_demux->s ) + i_size );
    }



On Mon, Mar 16, 2020, at 09:13, Steve Lhomme wrote:
> On 2020-03-16 9:03, Rémi Denis-Courmont wrote:
> > Le maanantaina 16. maaliskuuta 2020, 9.47.14 EET Steve Lhomme a écrit :
> >> On 2020-03-13 16:53, Thomas Guillem wrote:
> >>> Here is what I propose:
> >>>
> >>> static int ChunkSkip( demux_t *p_demux, uint32_t i_size )
> >>> {
> >>>
> >>>       i_size += i_size & 1;
> >>>       
> >>>       if( i_size >= 65536 )
> >>
> >> In general constant values are bad in the code.
> > 
> > [disputed]
> > 
> > If the same value must be reused several times, I can see the point in that.
> > (Though even then, we have plenty of, e.g., 80 instead of IPPORT_HTTP...)
> > 
> > But if the numebr is just an arbitrary limit that is only used once in the
> > code base, I really don't see the point in adding a symbolic constant. That
> > makes it confusingly look like it's actually a specified constant, which it is
> > not.
> > 
> > Especially here where it's obvious enough what the surrounding code does.
> 
> That's precisely because I don't understand where this value comes from 
> that I thought using a meaningful constant would help. Comments in the 
> code could help too.
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel