[vlc-devel] Re: ffmpeg encoder bugfix: almost fed libavcodec with a frame in the past

[Måns Rullgård]

Robert Stepanek <rost at lo-res.org> writes:

> Hello,
>
> On Jan 10, 2007, at 10:00 PM, Måns Rullgård wrote:
>
>> Robert Stepanek <rost at lo-res.org> writes:
>>>
>>> <PATCH BEGIN>
>>> --- vlc-trunk-vanilla/modules/codec/ffmpeg/encoder.c    2007-01-10
>>> 20:50:15.000000000 +0100
>>> +++ vlc-trunk/modules/codec/ffmpeg/encoder.c    2007-01-10
>>> 21:01:37.000000000 +0100
>>> @@ -802,7 +802,7 @@
>>>           frame.pts = AV_NOPTS_VALUE;
>>>       }
>>> -    if ( frame.pts != (signed int) AV_NOPTS_VALUE && frame.pts !=
>>> 0 )
>>> +    if ( (uint64_t)frame.pts != AV_NOPTS_VALUE && frame.pts != 0 )
>>>       {
>>>           if ( p_sys->i_last_pts == frame.pts )
>>>           {
>>> <PATCH END>
>>
>> This cast looks unnecessary.  The value of AV_NOPTS_VALUE requires 64
>> bits, so unless frame.pts is already a 64-bit number, it can't
>> possibly hold that value.  Besides, a cast that doesn't change
>> anything is useless and shouldn't be there at all in the first place.
>
> I see your point. Indeed the cast on uint64_t is unecessary (I
> checked it just now), except for gcc mocking about a comparison
> between signed and unsigned (encoder.c:805: warning: comparison
> between signed and unsigned; using gcc 4.0.3).

That begs the question, why is frame.pts signed?  FWIW, I personally
consider that warning quite useless.  Cluttering code with casts only
silence warnings does more harm than good.

> However the cast of AV_NOPTS_VALUE on a signed int breaks the code
> for sure on my machine (Linux 2.6.15-23-386) so at least this must be
> removed.

Oh yes.

-- 
Måns Rullgård
mru at inprovide.com

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html