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

Stefano Sabatini stefano.sabatini-lala
Fri Apr 23 01:37:55 CEST 2010


On date Wednesday 2010-04-21 02:11:16 +0200, Stefano Sabatini encoded:
> On date Tuesday 2010-04-20 10:02:52 +0200, Robert Kr?ger encoded:
> > 
> > On 19.04.2010, at 23:39, Stefano Sabatini wrote:
> > 
> > > On date Friday 2010-04-16 18:40:53 +0200, Robert Kr?ger encoded:
[...]
> > >> 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.
> > > 
> > 
> > 
> > makes sense. I would submit a patch. However, I saw that you already
> > factored out functions for the two cases where you already use "N/A"
> > and I'm not quite sure how to implement it in a way that is most
> > consistent. One function for each case seems overkill
> > (i.e. "nb_frames_string" analogous to "ts_value_string" and
> > "time_value_string") and a generic "int64_value_string" outputting
> > "N/A" for values > 0 feels odd. Any preferences? The only thing I
> > think is a no-brainer is to define "N/A" as a constant.
> 
> Yes the simplest solution looks the best one in this case.
> 
> Today I tried to figure out which is better between N/A and no
> printing at all, both solutions have their pros and cons, but at the
> end I slightly prefer the "N/A" solution as it conveys more
> information to the human reader, same for the other nb_frames pending
> patch.

After more thought:
1) 0:N with N != 0 is unknown
2) N:0 is undefined / Non/Acceptable = N/A

For the case 1) we may either don't print at all either print
"unknown" like it is done in other cases, so I tend to prefer this for
consistency, but I'd like to hear other opinions before to apply that
solution.

Regards.
-- 
FFmpeg = Friendly & Fanciful Mind-dumbing Puristic Eretic Gigant



More information about the ffmpeg-devel mailing list