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

[Robert Krüger]

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. 

> 
>> at least I guess that's what most people would think given the output of avcodec string but that assumption might be wrong.
>> 
>> if that is incorrect, would it make more sense to have ffprobe output N/A for sample aspect ratio if the numerator isn't set?
>> 
>> Another thing: Is there a reason that the values for sample_aspect_ratio are not filled for mjpeg files generated by ffmpeg or is this a bug that should be looked at?
> 
> a mjpeg file generated by ffmpeg does not contain a sample_aspect_ratio
> because its a file made of bytes and not a c structure.
> yes i know you meant something else but i dont know what
> 

never mind. your first comment probably answered that question.

Regards,

Robert