On Sun, 5 May 2019, Devin Heitmueller wrote:
Hello Marton,
On May 5, 2019, at 2:28 PM, Marton Balint <c...@passwd.hu> wrote:
On Wed, 1 May 2019, Marton Balint wrote:
Apparently in the new SDK one cannot query if VANC output is supported, so we
will fall back to non-VANC output if enabling the video output with VANC fails.
Fixes ticket #7867.
Applied.
I know it’s a bit late for a review given I’m only seeing this after
it’s been applied. However, has it been confirmed that the new logic
works with decklink SDKs older than version 11? If not, then the old
logic probably needs to stay and the new logic probably needs to be
#ifdef’d based on the SDK version. If we’re talking about increasing
the minimum SDK version for ffmpeg to build against, that’s also an
option (although given version 11 is very new I wouldn’t particularly be
in favor of that).
There were two code hunks. The one which called DoesSupportVideoMode was
an already ifdefed part to SDK version 11 only. The other hunk (the
fallback to non-VANC mode if enabling output with VANC fails) seemed
useful/harmless for all SDK versions, so I saw no reason to complicate it
with ifdefry.
Also, have you ascertained that the change in question works with more
than one model of card? What card did you encounter this issue with,
and what other cards did you test with? I’m just concerned about the
possibility that you committed a change to address an issue with some
particular card, and we don’t know what the effects are on other cards.
As far as I see the added code can't cause regressions for any HW/SDK
combo. If you see something in it where it can, then please let me know.
Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
