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

[Hendrik Leppkes]

On Tue, Apr 17, 2018 at 11:06 AM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Tue, Apr 17, 2018 at 08:32:57AM +0300, Timo Teras wrote:
>> On Tue, 17 Apr 2018 01:02:43 +0200
>> Michael Niedermayer <michael at niedermayer.cc> wrote:
>>
>> > On Mon, Apr 16, 2018 at 07:56:34PM +0200, Marton Balint wrote:
>> > >
>> > > 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.
>> >
>> > a very minor somewhat on topic nitpick, 0/0 would be mathamtically
>> > more correct as unknown than 0/1. If one doesnt immedeatly see why,
>> > one can look at width/height vs height/width to see one of many
>> > reasons why
>>
>> See my earlier patch that changes it to report as "N/A". This is what
>
> i meant the function. Which cannot output N/A as it outputs a "rational
> number" not a string.
> and for such numbers 0/0 closer represents undefined than 0/1 in a
> mathematical sense
>

Internally we have been using 0/1 for undefined aspect ratios since
like forever, I'm not sure why that was chosen, but maybe to avoid an
accidental division by zero?

- Hendrik