On 12/29/2017 1:20 PM, Devin Heitmueller wrote:
On Dec 29, 2017, at 4:09 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
2017-12-29 22:02 GMT+01:00 Devin Heitmueller <dheitmuel...@ltnglobal.com>:
On Dec 29, 2017, at 3:48 PM, Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmuel...@ltnglobal.com>:
+ /* FIXME: Should really rely on the coded_width but seems like that
+ is not accessible to libavdevice outputs */
+ if ((st->codecpar->width == 1280 && st->codecpar->height == 720) ||
+ (st->codecpar->width == 1920 && st->codecpar->height == 1080))
+ pkt->aspectRatio = ASPECT_16x9;
+ else
+ pkt->aspectRatio = ASPECT_4x3;
I most likely won't use this (and I have never seen a decklink card)
so please feel free to ignore:
Similar code has caused some trouble with mxf files, is there
really no saner solution? Like comparing what the actual aspect
ratio is more similar to? Is SAR really always 1 for decklink?
("All the world's a VAX.")
So this is definitely a confusing block of code, and you aren’t the first one to
ask about it (there were questions in the last round of review as well). The
aspect ratio referred to here is actually of the original coded video - not how
it’s supposed to be displayed. Hence, for example, 720x480 in widescreen
with a non-square PAR would still have the aspect ratio set to 4x3, since
that particular field describes the coded video (i.e. *not* how it’s supposed
to be rendered).
And a resolution of 1024x576 does not exist for decklink?
What about 1920x1088?
The Blackmagic cards have a fairly constrained set of modes which are supported (see
the definition of BMDDisplayMode in the Decklink SDK documentation). Neither of the
modes you mentioned are available for output, although I’m not sure what
st->codecpar->width would be set to if swscale is used between the decoder and
the output.
Bear in mind, I’m not particularly happy with how that block of code is done
either (which is why I marked it with a FIXME), but it’s probably good enough
for 95% of use cases.
Devin
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.
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.
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.
Aaron Levinson
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
