| Message ID | 1516469609-17820-1-git-send-email-raytiley@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi Ray, Thanks for your patch. A few questions: Could you confirm whether you are capturing via SDI or analog (i.e. composite/component)? Also what is the capturing device and SDK version you are using? I’ve found various bugs in the numbering of VANC lines in some cards, particularly with interlaced formats, and it would be good to understand if perhaps this is a card-specific issue. > On Jan 20, 2018, at 12:33 PM, Ray Tiley <raytiley@gmail.com> wrote: > > This changes the vertical blanking lines extracted for NTSC and 1080i > resolutions that in personal testing were required to extract closed > caption data from the decklink video frames. > > Additionally NTSC resolutions have the vanc data interleved between the uyvy > and not just the luma as in high definition resolutions. > > In my testing this allows a decklink card encoding valid NTSC and 1080i > closed captions to pass the caption data to the x264 encoder. > > Signed-off-by: Ray Tiley <raytiley@gmail.com> > --- > libavdevice/decklink_dec.cpp | 37 ++++++++++++++++++++++++++++++++----- > 1 file changed, 32 insertions(+), 5 deletions(-) > > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp > index 94dae26..bceced5 100644 > --- a/libavdevice/decklink_dec.cpp > +++ b/libavdevice/decklink_dec.cpp > @@ -67,8 +67,7 @@ typedef struct VANCLineNumber { > * another source during switching*/ > static VANCLineNumber vanc_line_numbers[] = { > /* SD Modes */ > - > - {bmdModeNTSC, 11, 19, 274, 282}, > + {bmdModeNTSC, 4, 21, 24, 284}, I hadn’t previously reviewed this table, but now that I look at it, I’m not confident your proposed patch is correct. VANC data generally cannot appear until after the switching line (through the first line of active video), which is on line 10 for NTSC 480i video. Could you elaborate on what equipment you have which is putting VBI data out on lines 4-10? Agreed though that the upper limit for field one is incorrect - it should be 21 as your patch proposed. Also, it’s highly unlikely that you really want to search line 24-284 for field 2 VBI data. Perhaps you meant 274-284? FYI: SMPTE RP 168-2009 is the normative reference for the location of switching lines across various video resolutions/framerates. > {bmdModeNTSC2398, 11, 19, 274, 282}, > {bmdModePAL, 7, 22, 320, 335}, > {bmdModeNTSCp, 11, -1, -1, 39}, > @@ -82,7 +81,7 @@ static VANCLineNumber vanc_line_numbers[] = { > {bmdModeHD1080p2997, 8, -1, -1, 42}, > {bmdModeHD1080p30, 8, -1, -1, 42}, > {bmdModeHD1080i50, 8, 20, 570, 585}, > - {bmdModeHD1080i5994, 8, 20, 570, 585}, > + {bmdModeHD1080i5994, 6, 30, 568, 595}, For 1080i the switching line is line 7, hence VBI should appear on line 8+. It’s possible for data to appear on line 7 if the transmitting equipment is misconfigured, and only some cards can actually capture that data (for example, the Decklink Duo cannot but the Decklink Duo2 can). Again, what is the signal source? > {bmdModeHD1080i6000, 8, 20, 570, 585}, > {bmdModeHD1080p50, 8, -1, -1, 42}, > {bmdModeHD1080p5994, 8, -1, -1, 42}, > @@ -92,7 +91,7 @@ static VANCLineNumber vanc_line_numbers[] = { > > {bmdModeHD720p50, 8, -1, -1, 26}, > {bmdModeHD720p5994, 8, -1, -1, 26}, > - {bmdModeHD720p60, 8, -1, -1, 26}, > + {bmdModeHD720p60, 7, -1, -1, 26}, Same questions as with the 1080i change above. Also, switching line location is the same for 720p/5994 and 720p/60, so presumably we would want to adjust both if it is actually needed. > > /* For all other modes, for which we don't support VANC */ > {bmdModeUnknown, 0, -1, -1, -1} > @@ -149,6 +148,30 @@ static void extract_luma_from_v210(uint16_t *dst, const uint8_t *src, int width) > } > } > > +static void unpack_v210(uint16_t *dst, const uint8_t *src, int width) > +{ > + int i; > + for (i = 0; i < width / 6; i++) { > + *dst++ = src[0] + ((src[1] & 3) << 8); > + *dst++ = (src[1] >> 2) + ((src[2] & 15) << 6); > + *dst++ = (src[2] >> 4) + ((src[3] & 63) << 4); > + > + *dst++ = src[4] + ((src[5] & 3) << 8); > + *dst++ = (src[5] >> 2) + ((src[6] & 15) << 6); > + *dst++ = (src[6] >> 4) + ((src[7] & 63) << 4); > + > + *dst++ = src[8] + ((src[9] & 3) << 8); > + *dst++ = (src[9] >> 2) + ((src[10] & 15) << 6); > + *dst++ = (src[10] >> 4) + ((src[11] & 63) << 4); > + > + *dst++ = src[12] + ((src[13] & 3) << 8); > + *dst++ = (src[13] >> 2) + ((src[14] & 15) << 6); > + *dst++ = (src[14] >> 4) + ((src[15] & 63) << 4); > + > + src += 16; > + } > +} > + > static uint8_t calc_parity_and_line_offset(int line) > { > uint8_t ret = (line < 313) << 5; > @@ -741,7 +764,11 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( > uint8_t *buf; > if (vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) { > uint16_t luma_vanc[MAX_WIDTH_VANC]; > - extract_luma_from_v210(luma_vanc, buf, videoFrame->GetWidth()); > + if (ctx->bmd_mode == bmdModeNTSC) { > + unpack_v210(luma_vanc, buf, videoFrame->GetWidth()); > + } else { > + extract_luma_from_v210(luma_vanc, buf, videoFrame->GetWidth()); > + } So I have to admit I find this bit odd. For composite video, the VBI data would always be in the luma region (in fact, most VBI decoders will strip out the chroma channel prior to processing). For SDI, it’s possible that VANC could appear in the luma or chroma channel, but it wouldn’t be be interleaved across both. I would be really interested to know what the source equipment is here, since I’m seeing a number of things that I believe would violate the various standards. While I’m a big believer in Postel’s Law, I would really like to know whether this is just some really bad output implementation, and if it’s open source then bugs should be raised against it rather than putting a bunch of hacks into ffmpeg. I guess it’s also possible the video went through a scaler which scaled the video but preserved the original VBI format (which would be bad but not terribly surprising). Cheers, Devin
On Mon, Jan 22, 2018 at 10:20 PM Devin Heitmueller < dheitmueller@ltnglobal.com> wrote: > Hi Ray, > > Thanks for your patch. A few questions: > > Could you confirm whether you are capturing via SDI or analog (i.e. > composite/component)? Also what is the capturing device and SDK version > you are using? I’ve found various bugs in the numbering of VANC lines in > some cards, particularly with interlaced formats, and it would be good to > understand if perhaps this is a card-specific issue. > > I'm capturing SDI, the only decklink card I have access to is the Decklink mini recorder, and I was using the latest sdk ( I can check exact version tomorrow during work.) It's definitely possible I expanded the search lines a bit too far, I was just hacking until I found the data. > > > On Jan 20, 2018, at 12:33 PM, Ray Tiley <raytiley@gmail.com> wrote: > > > > This changes the vertical blanking lines extracted for NTSC and 1080i > > resolutions that in personal testing were required to extract closed > > caption data from the decklink video frames. > > > > Additionally NTSC resolutions have the vanc data interleved between the > uyvy > > and not just the luma as in high definition resolutions. > > > > In my testing this allows a decklink card encoding valid NTSC and 1080i > > closed captions to pass the caption data to the x264 encoder. > > > > Signed-off-by: Ray Tiley <raytiley@gmail.com> > > --- > > libavdevice/decklink_dec.cpp | 37 ++++++++++++++++++++++++++++++++----- > > 1 file changed, 32 insertions(+), 5 deletions(-) > > > > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp > > index 94dae26..bceced5 100644 > > --- a/libavdevice/decklink_dec.cpp > > +++ b/libavdevice/decklink_dec.cpp > > @@ -67,8 +67,7 @@ typedef struct VANCLineNumber { > > * another source during switching*/ > > static VANCLineNumber vanc_line_numbers[] = { > > /* SD Modes */ > > - > > - {bmdModeNTSC, 11, 19, 274, 282}, > > + {bmdModeNTSC, 4, 21, 24, 284}, > > I hadn’t previously reviewed this table, but now that I look at it, I’m > not confident your proposed patch is correct. VANC data generally cannot > appear until after the switching line (through the first line of active > video), which is on line 10 for NTSC 480i video. Could you elaborate on > what equipment you have which is putting VBI data out on lines 4-10? > Agreed though that the upper limit for field one is incorrect - it should > be 21 as your patch proposed. > > Also, it’s highly unlikely that you really want to search line 24-284 for > field 2 VBI data. Perhaps you meant 274-284? > > FYI: SMPTE RP 168-2009 is the normative reference for the location of > switching lines across various video resolutions/framerates. > > For NTSC my source hardware was outputting on line 12, so I'll resubmit the patch with it adjusted after I dig up SMPTE RP 168-2009 and check the values.. The one I definitely had to modify in order for the search to find the data was bmdModeHD1080i5994, when I get to the office I'll hook up the signal to my scope and check the exact line and also run a test in 720p > > > {bmdModeNTSC2398, 11, 19, 274, 282}, > > {bmdModePAL, 7, 22, 320, 335}, > > {bmdModeNTSCp, 11, -1, -1, 39}, > > @@ -82,7 +81,7 @@ static VANCLineNumber vanc_line_numbers[] = { > > {bmdModeHD1080p2997, 8, -1, -1, 42}, > > {bmdModeHD1080p30, 8, -1, -1, 42}, > > {bmdModeHD1080i50, 8, 20, 570, 585}, > > - {bmdModeHD1080i5994, 8, 20, 570, 585}, > > + {bmdModeHD1080i5994, 6, 30, 568, 595}, > > For 1080i the switching line is line 7, hence VBI should appear on line > 8+. It’s possible for data to appear on line 7 if the transmitting > equipment is misconfigured, and only some cards can actually capture that > data (for example, the Decklink Duo cannot but the Decklink Duo2 can). > > Again, what is the signal source? > This makes sense, VBI was on line 12 for 1080i. The source was a Matrox DSX LE4. > > > {bmdModeHD1080i6000, 8, 20, 570, 585}, > > {bmdModeHD1080p50, 8, -1, -1, 42}, > > {bmdModeHD1080p5994, 8, -1, -1, 42}, > > @@ -92,7 +91,7 @@ static VANCLineNumber vanc_line_numbers[] = { > > > > {bmdModeHD720p50, 8, -1, -1, 26}, > > {bmdModeHD720p5994, 8, -1, -1, 26}, > > - {bmdModeHD720p60, 8, -1, -1, 26}, > > + {bmdModeHD720p60, 7, -1, -1, 26}, > > Same questions as with the 1080i change above. Also, switching line > location is the same for 720p/5994 and 720p/60, so presumably we would want > to adjust both if it is actually needed. > > > > > /* For all other modes, for which we don't support VANC */ > > {bmdModeUnknown, 0, -1, -1, -1} > > @@ -149,6 +148,30 @@ static void extract_luma_from_v210(uint16_t *dst, > const uint8_t *src, int width) > > } > > } > > > > +static void unpack_v210(uint16_t *dst, const uint8_t *src, int width) > > +{ > > + int i; > > + for (i = 0; i < width / 6; i++) { > > + *dst++ = src[0] + ((src[1] & 3) << 8); > > + *dst++ = (src[1] >> 2) + ((src[2] & 15) << 6); > > + *dst++ = (src[2] >> 4) + ((src[3] & 63) << 4); > > + > > + *dst++ = src[4] + ((src[5] & 3) << 8); > > + *dst++ = (src[5] >> 2) + ((src[6] & 15) << 6); > > + *dst++ = (src[6] >> 4) + ((src[7] & 63) << 4); > > + > > + *dst++ = src[8] + ((src[9] & 3) << 8); > > + *dst++ = (src[9] >> 2) + ((src[10] & 15) << 6); > > + *dst++ = (src[10] >> 4) + ((src[11] & 63) << 4); > > + > > + *dst++ = src[12] + ((src[13] & 3) << 8); > > + *dst++ = (src[13] >> 2) + ((src[14] & 15) << 6); > > + *dst++ = (src[14] >> 4) + ((src[15] & 63) << 4); > > + > > + src += 16; > > + } > > +} > > + > > static uint8_t calc_parity_and_line_offset(int line) > > { > > uint8_t ret = (line < 313) << 5; > > @@ -741,7 +764,11 @@ HRESULT > decklink_input_callback::VideoInputFrameArrived( > > uint8_t *buf; > > if (vanc->GetBufferForVerticalBlankingLine(i, > (void**)&buf) == S_OK) { > > uint16_t luma_vanc[MAX_WIDTH_VANC]; > > - extract_luma_from_v210(luma_vanc, buf, > videoFrame->GetWidth()); > > + if (ctx->bmd_mode == bmdModeNTSC) { > > + unpack_v210(luma_vanc, buf, > videoFrame->GetWidth()); > > + } else { > > + extract_luma_from_v210(luma_vanc, buf, > videoFrame->GetWidth()); > > + } > > So I have to admit I find this bit odd. For composite video, the VBI data > would always be in the luma region (in fact, most VBI decoders will strip > out the chroma channel prior to processing). For SDI, it’s possible that > VANC could appear in the luma or chroma channel, but it wouldn’t be be > interleaved across both. > > I would be really interested to know what the source equipment is here, > since I’m seeing a number of things that I believe would violate the > various standards. While I’m a big believer in Postel’s Law, I would > really like to know whether this is just some really bad output > implementation, and if it’s open source then bugs should be raised against > it rather than putting a bunch of hacks into ffmpeg. I guess it’s also > possible the video went through a scaler which scaled the video but > preserved the original VBI format (which would be bad but not terribly > surprising). > The source equipment was again a Matrox LE4. The source file was 1080i while I was testing the NTSC output, so it was perhaps a scaling issue. However the spec says that VANC data will be on the luma channel for high definition sources, so that is why I tried extracting it from the interleaved and was able to successfully parse the VANC packets. I chatted with someone in the video developers slack community and they suggested that for NTSC the data is indeed interleaved. I have access to some SD captioned files that I can play out my test hardware to eliminate scaling. -ray > > Cheers, > > Devin
Hi Ray, > On Jan 22, 2018, at 10:47 PM, Ray Tiley <raytiley@gmail.com> wrote: >> Could you confirm whether you are capturing via SDI or analog (i.e. >> composite/component)? Also what is the capturing device and SDK version >> you are using? I’ve found various bugs in the numbering of VANC lines in >> some cards, particularly with interlaced formats, and it would be good to >> understand if perhaps this is a card-specific issue. >> >> I'm capturing SDI, the only decklink card I have access to is the Decklink > mini recorder, and I was using the latest sdk ( I can check exact version > tomorrow during work.) It's definitely possible I expanded the search lines > a bit too far, I was just hacking until I found the data. Ok, so it’s possible that in the three cases you’ve changed, the start line was actually fine to begin with and the problem was just the end line was wrong. I could totally believe that. I can say that all three start lines are correct according to RP 168, but I would have to do the math to check the end lines (there was a time I could tell you from memory where the active video starts/ends for each video standard, but those days are long gone). > > The source equipment was again a Matrox LE4. The source file was 1080i > while I was testing the NTSC output, so it was perhaps a scaling issue. > However the spec says that VANC data will be on the luma channel for high > definition sources, so that is why I tried extracting it from the > interleaved and was able to successfully parse the VANC packets. Right. At least in libklvanc I don’t even bother looking at the chroma channel, since I’ve never seen a piece of equipment that puts any VANC there. For CEA-608 payloads and CEA-708 CDP packets, SMPTE ST 334-1:2015 Sec 4 clearly states that it must be in the luma channel for HD sources. > > I chatted with someone in the video developers slack community and they > suggested that for NTSC the data is indeed interleaved. Hmmm, this is a pretty ambiguous explanation. Got a link to the conversation? I also wouldn’t rule out a decklink bug, but it’s too early to make such an assertion. If you can get a dump of the raw VANC line data that I can review, that would be helpful. > > I have access to some SD captioned files that I can play out my test > hardware to eliminate scaling. Great. I think you’ve definitely found some issues - it’s just a question of whether we really have to deviate from the spec in order to work properly for your use case. Devin
On Mon, Jan 22, 2018 at 11:12 PM Devin Heitmueller < dheitmueller@ltnglobal.com> wrote: > Hi Ray, > > > On Jan 22, 2018, at 10:47 PM, Ray Tiley <raytiley@gmail.com> wrote: > >> Could you confirm whether you are capturing via SDI or analog (i.e. > >> composite/component)? Also what is the capturing device and SDK version > >> you are using? I’ve found various bugs in the numbering of VANC lines > in > >> some cards, particularly with interlaced formats, and it would be good > to > >> understand if perhaps this is a card-specific issue. > >> > >> I'm capturing SDI, the only decklink card I have access to is the > Decklink > > mini recorder, and I was using the latest sdk ( I can check exact version > > tomorrow during work.) It's definitely possible I expanded the search > lines > > a bit too far, I was just hacking until I found the data. > > Ok, so it’s possible that in the three cases you’ve changed, the start > line was actually fine to begin with and the problem was just the end line > was wrong. I could totally believe that. > > I can say that all three start lines are correct according to RP 168, but > I would have to do the math to check the end lines (there was a time I > could tell you from memory where the active video starts/ends for each > video standard, but those days are long gone). > > > > The source equipment was again a Matrox LE4. The source file was 1080i > > while I was testing the NTSC output, so it was perhaps a scaling issue. > > However the spec says that VANC data will be on the luma channel for high > > definition sources, so that is why I tried extracting it from the > > interleaved and was able to successfully parse the VANC packets. > > Right. At least in libklvanc I don’t even bother looking at the chroma > channel, since I’ve never seen a piece of equipment that puts any VANC > there. For CEA-608 payloads and CEA-708 CDP packets, SMPTE ST 334-1:2015 > Sec 4 clearly states that it must be in the luma channel for HD sources. > I'm reading 334-1:2017 Sec 4 "When the ANC packets defined in this standard are carried in a high definition signal, they shall be carried in the Y stream." I couldn't find anywhere in the document where it calls out standard definition Conversation is here: https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074 with kierank@obe.tv who I believe is in the ffmpeg-devel IRC. > > > > > I chatted with someone in the video developers slack community and they > > suggested that for NTSC the data is indeed interleaved. > > Hmmm, this is a pretty ambiguous explanation. Got a link to the > conversation? > > I also wouldn’t rule out a decklink bug, but it’s too early to make such > an assertion. > > If you can get a dump of the raw VANC line data that I can review, that > would be helpful. > > > > > I have access to some SD captioned files that I can play out my test > > hardware to eliminate scaling. > > Great. I think you’ve definitely found some issues - it’s just a question > of whether we really have to deviate from the spec in order to work > properly for your use case. > > Devin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
Hi Ray, > On Jan 22, 2018, at 11:20 PM, Ray Tiley <raytiley@gmail.com> wrote: > > I'm reading 334-1:2017 Sec 4 > "When the ANC packets defined in this standard are carried in a high > definition signal, they shall be carried in the Y stream." > > I couldn't find anywhere in the document where it calls out standard > definition Right, so my understanding was that allowing the ability to use both luma and chroma was a result of having less space in the VANC to hold data, compared to HD resolutions where that is much less likely to be an issue. That said, I’ve never seen an implementation that actually puts it in the chroma. > > Conversation is here: > https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074 <https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074> with > kierank@obe.tv <mailto:kierank@obe.tv> who I believe is in the ffmpeg-devel IRC. > Ah, ok. That’s Kieran. He’s really knowledgable in this area, although I cannot see the conversation you’ve linked to as it seems that room is only accessible by people with email accounts from certain domains. Devin
On Tue, Jan 23, 2018 at 9:07 AM Devin Heitmueller < dheitmueller@ltnglobal.com> wrote: > Hi Ray, > > > On Jan 22, 2018, at 11:20 PM, Ray Tiley <raytiley@gmail.com> wrote: > > > > I'm reading 334-1:2017 Sec 4 > > "When the ANC packets defined in this standard are carried in a high > > definition signal, they shall be carried in the Y stream." > > > > I couldn't find anywhere in the document where it calls out standard > > definition > > Right, so my understanding was that allowing the ability to use both luma > and chroma was a result of having less space in the VANC to hold data, > compared to HD resolutions where that is much less likely to be an issue. > That said, I’ve never seen an implementation that actually puts it in the > chroma. > > > > > Conversation is here: > > https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074 < > https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074> with > > kierank@obe.tv <mailto:kierank@obe.tv> who I believe is in the > ffmpeg-devel IRC. > > > > Ah, ok. That’s Kieran. He’s really knowledgable in this area, although I > cannot see the conversation you’ve linked to as it seems that room is only > accessible by people with email accounts from certain domains. > > Devin > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel 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. -ray
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
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@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 >
On Thu, 25 Jan 2018, Ray Tiley wrote: > On Tue, Jan 23, 2018 at 9:07 AM Devin Heitmueller < > dheitmueller@ltnglobal.com> wrote: > >> Hi Ray, >> >>> On Jan 22, 2018, at 11:20 PM, Ray Tiley <raytiley@gmail.com> wrote: >>> >>> I'm reading 334-1:2017 Sec 4 >>> "When the ANC packets defined in this standard are carried in a high >>> definition signal, they shall be carried in the Y stream." >>> >>> I couldn't find anywhere in the document where it calls out standard >>> definition >> >> Right, so my understanding was that allowing the ability to use both luma >> and chroma was a result of having less space in the VANC to hold data, >> compared to HD resolutions where that is much less likely to be an issue. >> That said, I’ve never seen an implementation that actually puts it in the >> chroma. >> >>> >>> Conversation is here: >>> https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074 < >> https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074> with >>> kierank@obe.tv <mailto:kierank@obe.tv> who I believe is in the >> ffmpeg-devel IRC. >>> >> >> Ah, ok. That’s Kieran. He’s really knowledgable in this area, although I >> cannot see the conversation you’ve linked to as it seems that room is only >> accessible by people with email accounts from certain domains. >> >> Devin >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > 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. Is interleaved VANC data is always the case for SD NTSC, or it depends on the equipment? Maybe we should add some auto detection, and check for the first VANC header. E.g. if (ctx->bmd_mode == bmdModeNTSC && (*(uint32_t*)buf & 0x3ffffffff == 0x3fffffc00)) unpack_v210 else unpack_luma_v210 Also, later on in the code, you only parse half of the decoded VANC. If you decode from both the luma and chroma, get_metadata should be called with frame width*2, no? Regards, Marton
On Sun, Jan 28, 2018 at 4:54 PM Marton Balint <cus@passwd.hu> wrote: > > On Thu, 25 Jan 2018, Ray Tiley wrote: > > > On Tue, Jan 23, 2018 at 9:07 AM Devin Heitmueller < > > dheitmueller@ltnglobal.com> wrote: > > > >> Hi Ray, > >> > >>> On Jan 22, 2018, at 11:20 PM, Ray Tiley <raytiley@gmail.com> wrote: > >>> > >>> I'm reading 334-1:2017 Sec 4 > >>> "When the ANC packets defined in this standard are carried in a high > >>> definition signal, they shall be carried in the Y stream." > >>> > >>> I couldn't find anywhere in the document where it calls out standard > >>> definition > >> > >> Right, so my understanding was that allowing the ability to use both > luma > >> and chroma was a result of having less space in the VANC to hold data, > >> compared to HD resolutions where that is much less likely to be an > issue. > >> That said, I’ve never seen an implementation that actually puts it in > the > >> chroma. > >> > >>> > >>> Conversation is here: > >>> https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074 < > >> https://video-dev.slack.com/archives/C0XKDDH5Y/p1516141555000074> with > >>> kierank@obe.tv <mailto:kierank@obe.tv> who I believe is in the > >> ffmpeg-devel IRC. > >>> > >> > >> Ah, ok. That’s Kieran. He’s really knowledgable in this area, > although I > >> cannot see the conversation you’ve linked to as it seems that room is > only > >> accessible by people with email accounts from certain domains. > >> > >> Devin > >> _______________________________________________ > >> ffmpeg-devel mailing list > >> ffmpeg-devel@ffmpeg.org > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > 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. > > Is interleaved VANC data is always the case for SD NTSC, or it depends on > the equipment? Maybe we should add some auto detection, and check for the > first VANC header. E.g. > > if (ctx->bmd_mode == bmdModeNTSC && > (*(uint32_t*)buf & 0x3ffffffff == 0x3fffffc00)) > unpack_v210 > else > unpack_luma_v210 > Sounds good to me. Will send patch shortly. > > Also, later on in the code, you only parse half of the decoded VANC. > If you decode from both the luma and chroma, get_metadata should be called > with frame width*2, no? > Good catch. I was testing with very small caption packets so I did not see it cut off. Will send patch shortly. > > Regards, > Marton > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp index 94dae26..bceced5 100644 --- a/libavdevice/decklink_dec.cpp +++ b/libavdevice/decklink_dec.cpp @@ -67,8 +67,7 @@ typedef struct VANCLineNumber { * another source during switching*/ static VANCLineNumber vanc_line_numbers[] = { /* SD Modes */ - - {bmdModeNTSC, 11, 19, 274, 282}, + {bmdModeNTSC, 4, 21, 24, 284}, {bmdModeNTSC2398, 11, 19, 274, 282}, {bmdModePAL, 7, 22, 320, 335}, {bmdModeNTSCp, 11, -1, -1, 39}, @@ -82,7 +81,7 @@ static VANCLineNumber vanc_line_numbers[] = { {bmdModeHD1080p2997, 8, -1, -1, 42}, {bmdModeHD1080p30, 8, -1, -1, 42}, {bmdModeHD1080i50, 8, 20, 570, 585}, - {bmdModeHD1080i5994, 8, 20, 570, 585}, + {bmdModeHD1080i5994, 6, 30, 568, 595}, {bmdModeHD1080i6000, 8, 20, 570, 585}, {bmdModeHD1080p50, 8, -1, -1, 42}, {bmdModeHD1080p5994, 8, -1, -1, 42}, @@ -92,7 +91,7 @@ static VANCLineNumber vanc_line_numbers[] = { {bmdModeHD720p50, 8, -1, -1, 26}, {bmdModeHD720p5994, 8, -1, -1, 26}, - {bmdModeHD720p60, 8, -1, -1, 26}, + {bmdModeHD720p60, 7, -1, -1, 26}, /* For all other modes, for which we don't support VANC */ {bmdModeUnknown, 0, -1, -1, -1} @@ -149,6 +148,30 @@ static void extract_luma_from_v210(uint16_t *dst, const uint8_t *src, int width) } } +static void unpack_v210(uint16_t *dst, const uint8_t *src, int width) +{ + int i; + for (i = 0; i < width / 6; i++) { + *dst++ = src[0] + ((src[1] & 3) << 8); + *dst++ = (src[1] >> 2) + ((src[2] & 15) << 6); + *dst++ = (src[2] >> 4) + ((src[3] & 63) << 4); + + *dst++ = src[4] + ((src[5] & 3) << 8); + *dst++ = (src[5] >> 2) + ((src[6] & 15) << 6); + *dst++ = (src[6] >> 4) + ((src[7] & 63) << 4); + + *dst++ = src[8] + ((src[9] & 3) << 8); + *dst++ = (src[9] >> 2) + ((src[10] & 15) << 6); + *dst++ = (src[10] >> 4) + ((src[11] & 63) << 4); + + *dst++ = src[12] + ((src[13] & 3) << 8); + *dst++ = (src[13] >> 2) + ((src[14] & 15) << 6); + *dst++ = (src[14] >> 4) + ((src[15] & 63) << 4); + + src += 16; + } +} + static uint8_t calc_parity_and_line_offset(int line) { uint8_t ret = (line < 313) << 5; @@ -741,7 +764,11 @@ HRESULT decklink_input_callback::VideoInputFrameArrived( uint8_t *buf; if (vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) { uint16_t luma_vanc[MAX_WIDTH_VANC]; - extract_luma_from_v210(luma_vanc, buf, videoFrame->GetWidth()); + if (ctx->bmd_mode == bmdModeNTSC) { + unpack_v210(luma_vanc, buf, videoFrame->GetWidth()); + } else { + extract_luma_from_v210(luma_vanc, buf, videoFrame->GetWidth()); + } txt_buf = get_metadata(avctx, luma_vanc, videoFrame->GetWidth(), txt_buf, sizeof(txt_buf0) - (txt_buf - txt_buf0), &pkt); }
This changes the vertical blanking lines extracted for NTSC and 1080i resolutions that in personal testing were required to extract closed caption data from the decklink video frames. Additionally NTSC resolutions have the vanc data interleved between the uyvy and not just the luma as in high definition resolutions. In my testing this allows a decklink card encoding valid NTSC and 1080i closed captions to pass the caption data to the x264 encoder. Signed-off-by: Ray Tiley <raytiley@gmail.com> --- libavdevice/decklink_dec.cpp | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-)