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).


- Derek
ffmpeg-devel mailing list

Reply via email to