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

[Steve Lhomme]

OK

On 2020-03-16 9:24, Thomas Guillem wrote:
>      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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>