[FFmpeg-devel] [PATCH] ffprobe: fix sample aspect ratio output when sample aspect ratio is not set

[Stefano Sabatini]

On date Friday 2010-04-16 18:40:53 +0200, Robert Kr?ger encoded:
> 
> On 16.04.2010, at 18:16, Michael Niedermayer wrote:
> 
> > On Thu, Apr 15, 2010 at 03:05:30PM +0200, Robert Kr?ger wrote:
> >> 
> >> On 15.04.2010, at 14:02, Michael Niedermayer wrote:
> >> 
> >>> On Thu, Apr 15, 2010 at 01:16:05PM +0200, Robert Kr?ger wrote:
> >>>> 
> >>>> the following patch fixes current behaviour where for example
> >>>> for all ffmpeg-generated mjpeg files sample aspect ratio is
> >>>> output as 0:1 and display aspect ratio is computed accordingly
> >>>> (i.e. wrong).
> >>> 
> >>> undefined and 1:1 are 2 different things
> >>> 
> >> 
> >> What currently happens is that obviously for some files with a
> >> sample aspect ratio of 1:1 the corresponding struct is filled
> >> correctly and for some it seems to be filled with 0:1.
> > 
> > If a field is filled incorrectly a bugreport is needed
> > 
> 
> OK, I'll file one, then you can decide if it's really a bug and
> close it if it's not.
> 
> > 
> >> I looked at the way this is treated in avcodec_string in utils.c
> >> and line 857 suggests that it treats a zero numerator for sample
> >> aspect ratio as 1:1
> > 
> > no it does not
> > 
> 
> You're probably right although I would suspect that there are tons
> of programs out there which parse the output of ffmpeg to determine
> the display size of the video which will have an incorrect result
> here but that's beside the point, I know. What I would like someone
> to decide is how ffprobe should communicate the sample aspect ratio
> (and consequently the display aspect ratio as well) not being
> available (i.e. numerator being zero, at least that criterion is
> used in other places in ffmpeg code)
> 
> 1) output "N/A" for both values
> 2) don't output those key value pairs at all
> 3) output what's in the struct (e.g. 0:1 in my case) for both like it does now
> 
> I think 3 does not make sense and would submit a patch for 1 or 2 if
> someone (Stefano or you?) tells me which.

I have a slight preference for solution 1), and we're already using
"N/A" at least in another case (for PTS == AV_NOPTS_VALUE), so if
there are not other opinions that should be fine.

Regards.
-- 
FFmpeg = Free & Furious Multimedia Plastic Elegant Guru