[FFmpeg-devel] [PATCH] Ensure sample_aspect_ratio.den is nonzero

[David Conrad]

On Apr 21, 2010, at 3:04 PM, Michael Niedermayer wrote:

> On Wed, Apr 21, 2010 at 10:02:43AM -0400, David Conrad wrote:
>> On Apr 21, 2010, at 9:57 AM, Michael Niedermayer wrote:
>> 
>>> On Wed, Apr 21, 2010 at 09:37:29AM -0400, David Conrad wrote:
>>>> On Apr 21, 2010, at 9:11 AM, Michael Niedermayer wrote:
>>>> 
>>>>> On Wed, Apr 21, 2010 at 06:32:58AM -0400, David Conrad wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> Currently no decoders check for validity of sample_aspect_ratio, specifically, whether sample_aspect_ratio.den is nonzero. This leads to an issue with theora files, which have 0/0 written a lot of the time, causing ffmpeg to calculate an aspect ratio of 4/1 after some math involving NaNs.
>>>>> 
>>>>> some?
>>>>> elaborate please
>>>> 
>>>> av_q2d(sar) return NaN for 0/0, ffmpeg does some multiplications so it's still a NaN, then av_d2q returns 4/1 for a NaN due to how it's implemented.
>>>> 
>>>>>> Since "unknown" is the only valid interpretation of a denominator of zero, this adds a check to both avcodec_open() and avcodec_decode_video2() to set {0,1} which is a safer value to give to user applications.
>>>>>> 
>>>>>> Alternatively, I could put the check in each codec that sets it directly from the bitstream, or clarify the API that a zero denominator is allowed and fix ffmpeg.
>>>>> 
>>>>> I would have assumed that 0/0 is allowed and meant unknown aspect but i
>>>>> need to first know where the problem you are working on is to be able
>>>>> to decide what would be the best approuch to fix that or if we should
>>>>> avoid 0/0
>>>> 
>>>> Alternate patch to return 0/1 from av_d2q for NaN attached, which also fixes the issue.
>>> 
>>> 0/1 is not nan 0/1 is 0
>>> 
>>> nan should become 0/0 again
>> 
>> That works too, I was just worried about random people dividing by 0.
> 
> try this argument when talking to some mathematican
> 
> 
>> 
> 
>> commit 1c361e9c492a6834dc720013e9ceee6d4b23a029
>> Author: David Conrad <lessen42 at gmail.com>
>> Date:   Wed Apr 21 09:35:39 2010 -0400
>> 
>>    Convert NaN to 0/0 in av_d2q
>> 
>> diff --git a/libavutil/rational.c b/libavutil/rational.c
>> index 3217b7d..3e8b885 100644
> 
> patch looks ok

Seem to have missed applying this, applied.