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 ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
