Apologies for attaching the patch, still trying to figure out patches, didn't know how to send the patch and include info in the email unrelated to commit message / change.
The unpack_v210 is only called for SD resolutions ,specifically NTSC which is 720 wide. unpack_v210 should never be called for HD resolutions as that would violate the spec in which all the vanc should be in the lama. So MAX_WIDTH_VANC being 1920 is wide enough to hold a single line of unpacked SD resolution. But probably better to be safe then risk an overflow. I see a few options. Increase luma_vanc to be MAX_WIDTH_VANC * 3, but a guard in the unpack_v210 (should it just return early, log a warning), or do both. Any preferences. C is not my day to day language so let me know what' best practice and I'll get the patch fixed up. -ray On Thu, Jan 25, 2018 at 9:47 AM Devin Heitmueller < dheitmuel...@ltnglobal.com> wrote: > Hi Ray, > > > > > Please find updated patch attatched. I reverted the vanc lines changes > and > > found that all my tests worked as expected, so not sure what was wrong w/ > > my original test. The need to extract vanc from the entire line vs just > the > > luma in NTSC is still required. > > It’s helpful if in the future you could not do patches as attachments. It > makes it harder for people to comment on them. > > Glad to hear you didn’t need to adjust any of the VANC line definitions in > order to work properly. I think they do still need some more review, but > at least we don’t need to commit to any values at this time which would > violate the spec. > > Regarding the luma/chroma extraction, I haven’t looked at your code too > closely, but isn’t the destination array too small? If MAX_WIDTH_VANC is > 1920, presumably intended to be the number of pixels, then you would need > three times as many uint16_t values in your destination array if you wanted > Y, U, and V, or else you would overflow the buffer. Right? In either > case, you probably want some bounds protection to ensure GetWidth() never > returns a size greater than your destination array. > > Also, could you send me a copy of the array of V210 bytes you are testing > with (i.e. just jam a printf loop into the code and dump the whole thing > out)? Would be useful to have it here so I can ensure that libklvanc works > properly as well (and add it to the list of test vectors I bundle with the > library). If you can’t that’s fine - I’ll eventually get around to doing > some SD testing here. > > Devin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
