[FFmpeg-devel] [PATCH] avdevice/decklink: fix checking video mode in SDK version 11

[Marton Balint]

On Sun, 5 May 2019, Devin Heitmueller wrote:

> Hello Marton,
>
>> On May 5, 2019, at 2:28 PM, Marton Balint <cus at 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