[FFmpeg-devel] [PATCH 5/6] Add suppoort for using libklvanc from within decklink capture module
derek.buitenhuis at gmail.com
Wed Nov 29 22:34:11 EET 2017
On 11/29/2017 7:17 PM, Devin Heitmueller wrote:
>> Is there a reason we shouldn't fail hard here?
> Not really. The parser will log an error if the callback returns a nonzero value, but beyond the return value isn’t actively used. That said, no objection to having it return -1 for clarity.
I have no strong feelings either way.
>> I thought C++ didn't have designated initializers? Maybe my C++ is rusty.
> Clang didn’t complain, and g++ only complains if you put them in a non-default order (i.e. "non-trivial designated initializers not supported"). The designated initializers improve readability but aren’t required (since already put the items in the default order). If there’s a portability concern then I can get rid of them.
The internet seems to claim it's a GNU extension. Will this code ever possibly
be built with something that isn't GCC or Clang?
>> Same for other occurrences.
> I’m sorry, but what other occurrences? I don’t see any other instances in this patch where designated initializers are used — or did I misunderstand your comment?
My mail must have got mangled while I was editing it. Ignore this, I think.
>> Is buf guaranteed to be properly aligned for this, or will cause aliasing problems?
> Hmm, good question. The start of each line will always be aligned on a 48 byte boundary as a result of how the decklink module manages it’s buffers, but I agree that this block of code is a bit messy and needs some cleanup (hence the TODO).
Is this aligment a guarantee by the module?
> I suspect the original routine was cribbed from OBE (with portions derived from ffmpeg’s v210dec), and the assembly version of the same function probably isn’t as forgiving (although libklvanc doesn’t provide an assembly implementation as this routine isn’t particularly performance sensitive).
More information about the ffmpeg-devel