[FFmpeg-devel] [PATCH 1/1] [RFC] ffprobe: report DAR even if SAR is undefined

[Marton Balint]

On Mon, 16 Apr 2018, Timo Teras wrote:

> On Sun, 15 Apr 2018 16:42:01 +0200 (CEST)
> Marton Balint <cus at passwd.hu> wrote:
>
>> On Sun, 15 Apr 2018, Timo Teräs wrote:
>> 
>> > Calculate DAR with assumed SAR 1:1 when SAR is undefined. Same
>> > assumption is done in ffplay to create the play window. Usually
>> > DAR is more useful metadata than SAR when e.g. choosing which
>> > media of multiple versions to use to fit the display.
>> 
>> I don't think it's good idea to generally assume 1:1
>> display_aspect_ratio for every undefined sample aspect ratio. It
>> depends heavily on your actual use case. If MOV/MP4 specifies that
>> 1:1 SAR should be used, then maybe you should fix
>> av_guess_sample_aspect_ratio instead, and return 1:1 there if the
>> format context is MOV/MP4. You may add a demuxer (AVFMT) flag to
>> signal that such behaviour should be used, and
>> av_guess_sample_aspect_ratio can check for that demuxer flag.
>
> Looking at code, av_guess_sample_aspect_ratio() is used only in ffplay
> and ffprobe.
>
> ffplay implicitly assumes undefined SAR is 1:1 to create the playback
> window properly; this happens in calculate_display_rect() when
> "bad" aspect_ratio is reset to 1.0.
>
> I would expect same logic would have been useful in ffprobe. This would
> help to report back to user what ffplay is going to do with the video.
> Or at least give a hint on how to categorize the clip. SAR 1:1 is
> pretty good guess for most formats.

I really don't see why don't you fix your application instead which parses 
ffprobe output? If you see N/A aspect ratio, use 1:1.

To be frank, I am not sure if ffprobe should use av_guess_aspect_ratio 
when it displays stream metadata. It is only there now to av_reduce the 
aspect ratios and to sanitize some invalid aspect ratios to 0/1. FFprobe's 
job is to return stream metadata as is, not to make guesses.

>
> For this reason, my preferred solution was to patch ffprobe so we can
> give a guess for all files. If the above patch is not a good idea, how
> about adding new "effective_{sample,display}_aspect_ratio" fields? Or
> just a flag "aspect_ratio_guessed" to tell if it's not defined in the
> file?

Effective is not a good name here. Use best_effort_* or 
guessed_* if you have to, but I don't see the much benefit in adding extra 
fields to ffprobe output for such simple and trivial heuristics. Like I 
said, it's not ffprobe's job. The next guy will want to add a 
guessed_color_primaries based on resolution, and I can think of more 
complicated guesses :)

>
> I would prefer not to do any file type specific special handling if
> possible. However, if that's the only acceptable solution, I'm happy to
> implement that too. But then I'd prefer to have a 'no default SAR of
> 1:1' flags so file formats can inhibit the assumption instead of
> explicitly needing to enable it. Is there any formats where this would
> be useful? Or how about just making av_guess_sample_aspect_ratio()
> return 1:1 in case nothing better exists? It's called "guess" after all
> and is used in ffplay/ffprobe only...

Opt in for MOV/MP4 is required because *the specs says* that renderers 
should behave this way. So the aspect ratio guess is based on 
something, and not a wild guess. I think most of the other format specs 
does not say such things.

Regards,
Marton