| Message ID | 1517193605-12417-1-git-send-email-raytiley@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Jan 28, 2018 at 9:40 PM Ray Tiley <raytiley@gmail.com> wrote: > This changes how NTSC VANC is extracted from the buffer. In NTSC 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 closed > captions to pass the caption data to the x264 encoder. > > Updated with rewviews from Devon Heitmueller and Marton Balint. > > Signed-off-by: Ray Tiley <raytiley@gmail.com> > --- > libavdevice/decklink_dec.cpp | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp > index 94dae26..c7811eb 100644 > --- a/libavdevice/decklink_dec.cpp > +++ b/libavdevice/decklink_dec.cpp > @@ -149,6 +149,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 +765,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); > } > -- > 2.7.4 > > Anything else I need to do for this patch? Thanks -ray
On Mon, 5 Feb 2018, Ray Tiley wrote: > On Sun, Jan 28, 2018 at 9:40 PM Ray Tiley <raytiley@gmail.com> wrote: > >> This changes how NTSC VANC is extracted from the buffer. In NTSC 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 closed >> captions to pass the caption data to the x264 encoder. >> >> Updated with rewviews from Devon Heitmueller and Marton Balint. >> >> Signed-off-by: Ray Tiley <raytiley@gmail.com> >> --- >> libavdevice/decklink_dec.cpp | 30 +++++++++++++++++++++++++++++- >> 1 file changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp >> index 94dae26..c7811eb 100644 >> --- a/libavdevice/decklink_dec.cpp >> +++ b/libavdevice/decklink_dec.cpp >> @@ -149,6 +149,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 +765,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); >> } >> -- >> 2.7.4 >> >> > > Anything else I need to do for this patch? I thought you are working on an updated patch, or I just missed the new one? The email you replied to here definitely contains the old, without interleaved VANC autodetection or proper width for get_metadata. Regards, Marton
Hello Marton,
> I thought you are working on an updated patch, or I just missed the new one? The email you replied to here definitely contains the old, without interleaved VANC autodetection or proper width for get_metadata.
I did some digging into this after discussion with Ray (both by reviewing the SMPTE specs as well as doing some testing here with my Sencore MRD4400 decoder). The use of VANC over both luma and chroma is because it’s an SD format. Hence it will only be present in specific modes. When doing HD formats, the VANC packets will be exclusively found in the luma region.
That said, the list of modes should probably be expanded to include all the SD resolutions (although you’re unlikely to see CEA-708 over non-NTSC streams). However I don’t think it would be a good idea to attempt to ‘autodetect” by applying both algorithms over all VANC lines regardless of mode.
As a result of these findings I’ll also be making a tweak to libklvanc, adding an entry point to the library which simply takes the entire V210 line, and the library can apply the appropriate colorspace conversions internally (hence eliminating the need for this sort of decision making to occur in the application).
Devin
On Mon, 5 Feb 2018, Devin Heitmueller wrote: > Hello Marton, > >> I thought you are working on an updated patch, or I just missed the new >> one? The email you replied to here definitely contains the old, without >> interleaved VANC autodetection or proper width for get_metadata. > > I did some digging into this after discussion with Ray (both by > reviewing the SMPTE specs as well as doing some testing here with my > Sencore MRD4400 decoder). The use of VANC over both luma and chroma is > because it’s an SD format. Hence it will only be present in specific > modes. When doing HD formats, the VANC packets will be exclusively > found in the luma region. Have you found which standard contains reference to interleaved VANC? > > That said, the list of modes should probably be expanded to include all > the SD resolutions (although you’re unlikely to see CEA-708 over > non-NTSC streams). However I don’t think it would be a good idea to > attempt to ‘autodetect” by applying both algorithms over all VANC lines > regardless of mode. I think the plan was to check if the mode is NTSC _and_ the first VANC header is present in an interleaved way. So the HD modes would remain as before. I only found ITU-R BT.1364-3 which states that luma and chroma are separate VANC spaces, so that is why I thought autodetection for even NTSC would make it more compatible with newer equipment respecting this recommendation. If you think this is unnecesary then so be it, I have no experience with real life baseband NTSC signals. Regards, Marton
Hi Marton, > > Have you found which standard contains reference to interleaved VANC? So SMPTE falls back onto the earlier standards for digital video bitstreams as defined in ITU BT.656 (for SD) and BT.1120 (for HD). > >> >> That said, the list of modes should probably be expanded to include all the SD resolutions (although you’re unlikely to see CEA-708 over non-NTSC streams). However I don’t think it would be a good idea to attempt to ‘autodetect” by applying both algorithms over all VANC lines regardless of mode. > > I think the plan was to check if the mode is NTSC _and_ the first VANC header is present in an interleaved way. So the HD modes would remain as before. > > I only found ITU-R BT.1364-3 which states that luma and chroma are separate VANC spaces, so that is why I thought autodetection for even NTSC would make it more compatible with newer equipment respecting this recommendation. I assume you’re referring to this specific excerpt from ITU BT.1364 Sec 4: "In interfaces conforming to Recommendation ITU-R BT.1120, the data words corresponding to the luminance and colour-difference channels are considered to form two independent ancillary data spaces, each of which begins with its own timing reference signal (and line number and CRCC)." The key here is that this paragraph refers exclusively to interfaces conforming to BT.1120. BT.1120 is bitstream format for HDTV interfaces, and doesn’t apply to standard definition. Cases where we’re talking about standard definition (as specified in BT.656 and BT.799) don’t treat luma and chroma as two independent ancillary data spaces. Now Ray pointed out that SMPTE ST 334-1:2015 states the following in 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.” This could certainly be considered ambiguous since it doesn’t state explicitly how ANC packets in standard definition should be carried. I’m not willing to make that leap though given I’ve now looked at a couple of pieces of commercial gear, what Kieran’s OBE code has been doing for years without issue, as well as the contents of the ITU specs. All that said, if somebody wants to show me a VANC dump from a piece of commercially available hardware which does treat the channels separately in standard definition, I would certainly be willing to change my stance. I would rather wait until that happens though rather than have code in ffmpeg which has never been exercised with any real input (and likely never will be). Devin
On Mon, 5 Feb 2018, Devin Heitmueller wrote: > Hi Marton, > >> >> Have you found which standard contains reference to interleaved VANC? > > So SMPTE falls back onto the earlier standards for digital video bitstreams as defined in ITU BT.656 (for SD) and BT.1120 (for HD). > >> >>> >>> That said, the list of modes should probably be expanded to include all the SD resolutions (although you’re unlikely to see CEA-708 over non-NTSC streams). However I don’t think it would be a good idea to attempt to ‘autodetect” by applying both algorithms over all VANC lines regardless of mode. >> >> I think the plan was to check if the mode is NTSC _and_ the first VANC header is present in an interleaved way. So the HD modes would remain as before. >> >> I only found ITU-R BT.1364-3 which states that luma and chroma are separate VANC spaces, so that is why I thought autodetection for even NTSC would make it more compatible with newer equipment respecting this recommendation. > > I assume you’re referring to this specific excerpt from ITU BT.1364 Sec 4: > > "In interfaces conforming to Recommendation ITU-R BT.1120, the data words corresponding to the luminance and colour-difference channels are considered to form two independent ancillary data spaces, each of which begins with its own timing reference signal (and line number and CRCC)." > > The key here is that this paragraph refers exclusively to interfaces conforming to BT.1120. BT.1120 is bitstream format for HDTV interfaces, and doesn’t apply to standard definition. Cases where we’re talking about standard definition (as specified in BT.656 and BT.799) don’t treat luma and chroma as two independent ancillary data spaces. > > Now Ray pointed out that SMPTE ST 334-1:2015 states the following in 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.” > > This could certainly be considered ambiguous since it doesn’t state explicitly how ANC packets in standard definition should be carried. I’m not willing to make that leap though given I’ve now looked at a couple of pieces of commercial gear, what Kieran’s OBE code has been doing for years without issue, as well as the contents of the ITU specs. > > All that said, if somebody wants to show me a VANC dump from a piece of commercially available hardware which does treat the channels separately in standard definition, I would certainly be willing to change my stance. I would rather wait until that happens though rather than have code in ffmpeg which has never been exercised with any real input (and likely never will be). Ok, then I guess only the width needs fixing, which is doubled if the source is interleaved. Thanks, Marton
On Mon, Feb 5, 2018 at 7:32 PM Marton Balint <cus@passwd.hu> wrote: > > > On Mon, 5 Feb 2018, Devin Heitmueller wrote: > > > Hi Marton, > > > >> > >> Have you found which standard contains reference to interleaved VANC? > > > > So SMPTE falls back onto the earlier standards for digital video > bitstreams as defined in ITU BT.656 (for SD) and BT.1120 (for HD). > > > >> > >>> > >>> That said, the list of modes should probably be expanded to include > all the SD resolutions (although you’re unlikely to see CEA-708 over > non-NTSC streams). However I don’t think it would be a good idea to > attempt to ‘autodetect” by applying both algorithms over all VANC lines > regardless of mode. > >> > >> I think the plan was to check if the mode is NTSC _and_ the first VANC > header is present in an interleaved way. So the HD modes would remain as > before. > >> > >> I only found ITU-R BT.1364-3 which states that luma and chroma are > separate VANC spaces, so that is why I thought autodetection for even NTSC > would make it more compatible with newer equipment respecting this > recommendation. > > > > I assume you’re referring to this specific excerpt from ITU BT.1364 Sec > 4: > > > > "In interfaces conforming to Recommendation ITU-R BT.1120, the data > words corresponding to the luminance and colour-difference channels are > considered to form two independent ancillary data spaces, each of which > begins with its own timing reference signal (and line number and CRCC)." > > > > The key here is that this paragraph refers exclusively to interfaces > conforming to BT.1120. BT.1120 is bitstream format for HDTV interfaces, > and doesn’t apply to standard definition. Cases where we’re talking about > standard definition (as specified in BT.656 and BT.799) don’t treat luma > and chroma as two independent ancillary data spaces. > > > > Now Ray pointed out that SMPTE ST 334-1:2015 states the following in 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.” > > > > This could certainly be considered ambiguous since it doesn’t state > explicitly how ANC packets in standard definition should be carried. I’m > not willing to make that leap though given I’ve now looked at a couple of > pieces of commercial gear, what Kieran’s OBE code has been doing for years > without issue, as well as the contents of the ITU specs. > > > > All that said, if somebody wants to show me a VANC dump from a piece of > commercially available hardware which does treat the channels separately in > standard definition, I would certainly be willing to change my stance. I > would rather wait until that happens though rather than have code in ffmpeg > which has never been exercised with any real input (and likely never will > be). > > Ok, then I guess only the width needs fixing, which is doubled if the > source is interleaved. > Thanks, > Marton > Just sent the updated patch (hopefully correctly). Sorry about the outdated one before. Thanks -ray > _______________________________________________ > 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..c7811eb 100644 --- a/libavdevice/decklink_dec.cpp +++ b/libavdevice/decklink_dec.cpp @@ -149,6 +149,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 +765,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 how NTSC VANC is extracted from the buffer. In NTSC 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 closed captions to pass the caption data to the x264 encoder. Updated with rewviews from Devon Heitmueller and Marton Balint. Signed-off-by: Ray Tiley <raytiley@gmail.com> --- libavdevice/decklink_dec.cpp | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)