[FFmpeg-devel] [PATCH] avdevice/decklink_dec: Extract 1080i and NTSC VANC

[Ray Tiley]

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