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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100421/6401d5e8/attachment.pgp>