[FFmpeg-devel] [PATCH] avdevice/decklink_dec: extracting and outputing klv from vanc

[Marton Balint]

On Mon, 15 Jun 2020, Zivkovic, Milos wrote:

> Hello,
>
> I appreciate the comments.
>
> Missing docs for the new option.
>>
>
> Does this refer to .texi files? I'll update the docs.

Yes.

>
>
>> Avoid these if possible. Yes, this is C++ code but only because the
>> decklink
>> API requires it. Also it gives me a compile error, and I am not familiar
>> enough
>> with C++ to figure out what is going on
>>
>
> It makes everything much easier (just like using std::atomic for ref
> counting does).
> It also doesn't change the needed version of C++ since std::atomic is
> available in C++11 and above.
> However, that error message is concerning and I'd like to fix it. Which
> compiler is it?

GCC 7.5.0, full compiler log is attached.

>
> You only use this to avoid calling ->Release() right? If that is the case,
>> then
>> I'd avoid this as well, I am not sure about the compiler support of such
>> templating.
>>
>
> Yes, it's used to avoid manually releasing COM pointers which can become
> quite a hassle especially if multiple COM objects are involved.
> Error handling is simplified as well since there's no need to worry which
> COM object(s) should be released.
> As far as compiler support goes, this is the most basic C++ template that's
> supported by basically every C++ compiler in the last 20+ years.
> I'm willing to remove it if you still feel strongly that it's unnecessary.

Yes please.

>
> This interface is only available from DeckLink SDK 10.10 or so, but you
>> have not
>> bumped the version requirement in configure.
>>
>
> Wasn't sure which files should be edited for this.
> I'll get the exact SDK version that introduces it and bump it in configure.
>
> Is the sorting really needed? Aren't they supposed to be sorted in the
>> VANC?
>>
>
> Well, they should be, but it's not guaranteed so this is just a safety net.
> If it was guaranteed there would be no need for PSC fields.
>
> Maybe avoid the error message and increase ctx->dropped like other similar
>> code
>> does it.
>>
>
> Will do.
>
> Mixed code and declaration.
>>
>
> I'll move that block below the declaration.
>
> Lose the error message for ENOMEM.
>>
>
> I was just following what the other code that adds streams does.
>
> AV_OPT_TYPE_BOOL
>>
>
> I'll change that asap.
>
> Thanks,
> Milos

Thanks,
Marton
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: compile_error.log
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200615/6ea4be49/attachment.ksh>