[FFmpeg-devel] [PATCH 2/8] decklink: Add support for output of Active Format Description (AFD)

[Devin Heitmueller]

Hello Aaron,

Thanks for the feedback.  Comments inline.

On Sat, Dec 30, 2017 at 12:34 AM, Aaron Levinson
<alevinsn_dev at levland.net> wrote:

> Technically, there are a number of 2K and 4K video modes supported by some
> DeckLink cards that have a 16x9 aspect ratio as well.  This code would treat
> such video modes at 4:3.  The various 4K DCI video modes supported by
> Blackmagic use an aspect ratio of 256:135, although perhaps it is sufficient
> to treat those as 16:9.  Perhaps the logic should be, anything with a width
> of 1280 or greater (or a height of 720 or greater) is 16:9--everything else
> can be treated as 4:3.

While SMPTE 2016-1 doesn't support anything above 1080p, I'm not
against a bit of future-proofing, in particular given this code is
already pretty ugly.

My thought would be to do (width / height) and if the value is (<=
1.33) then treat it as 4x3.  Otherwise treat it as 16:9.  That seems
general enough, and will ensure that any higher resolutions which get
introduced won't require a code change.

> Admittedly, libklvanc may not support VANC for 2K and 4K video anyway, but
> the Blackmagic devices do support it, so if you want to rule those out, then
> some explicit code to only support up to 720p/1080i (maybe 1080p?) probably
> ought to be added.

I'm not against such a change, given I haven't done any 4K testing.
Once some validation has been done then we can look to pull the check
out.

> On a separate note, based on a similar review I did awhile back, if this set
> of patches only supports a specific set of video modes, then there probably
> ought to be checks to make sure the code changes are only exercised for
> those video modes.

The goal at this point is to support all the video modes typically
seen in the broadcast industry.  We've got a flag called
"ctx->supports_vanc" which drives whether any of the VANC processing
happens, and I guess we could have some sort of list of supported
formats and the rest would have it disabled.  Then we an add the 4K
and RGB modes once they get some attention.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com