Hello Aaron, Thanks for the feedback. Comments inline.
On Sat, Dec 30, 2017 at 12:34 AM, Aaron Levinson <alevinsn_...@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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
