On 6/30/2017 5:05 PM, Marton Balint wrote:
This also add supports for 4K DeckLink cards because they always output the
ancillary data in 10-bit.
Signed-off-by: Marton Balint <c...@passwd.hu>
---
doc/indevs.texi | 4 ++--
libavdevice/decklink_dec.cpp | 56 +++++++++++++++++++++++++++++++-------------
2 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/doc/indevs.texi b/doc/indevs.texi
index 51c304f3ec..330617a7c9 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -250,8 +250,8 @@ specifically lines 6 to 22, and lines 318 to 335. Line 6 is
the LSB in the mask.
Selected lines which do not contain teletext information will be ignored. You
can use the special @option{all} constant to select all possible lines, or
@option{standard} to skip lines 6, 318 and 319, which are not compatible with
all
-receivers. Capturing teletext only works for SD PAL sources in 8 bit mode.
-To use this option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
+receivers. Capturing teletext only works for SD PAL sources. To use this
+option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
@item channels
Defines number of audio channels to capture. Must be @samp{2}, @samp{8} or
@samp{16}.
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 39974e3ff4..88af01be70 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -54,21 +54,40 @@ static uint8_t calc_parity_and_line_offset(int line)
return ret;
}
-int teletext_data_unit_from_vbi_data(int line, uint8_t *src, uint8_t *tgt)
+static void fill_data_unit_head(int line, uint8_t *tgt)
+{
+ tgt[0] = 0x02; // data_unit_id
+ tgt[1] = 0x2c; // data_unit_length
+ tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
+ tgt[3] = 0xe4; // framing code
+}
+
+static uint8_t* teletext_data_unit_from_vbi_data(int line, uint8_t *src,
uint8_t *tgt, vbi_pixfmt fmt)
{
vbi_bit_slicer slicer;
- vbi_bit_slicer_init(&slicer, 720, 13500000, 6937500, 6937500, 0x00aaaae4, 0xffff, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, VBI_PIXFMT_UYVY);
+ vbi_bit_slicer_init(&slicer, 720, 13500000, 6937500, 6937500, 0x00aaaae4,
0xffff, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, fmt);
if (vbi_bit_slice(&slicer, src, tgt + 4) == FALSE)
- return -1;
+ return tgt;
- tgt[0] = 0x02; // data_unit_id
- tgt[1] = 0x2c; // data_unit_length
- tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
- tgt[3] = 0xe4; // framing code
+ fill_data_unit_head(line, tgt);
- return 0;
+ return tgt + 46;
+}
+
+static uint8_t* teletext_data_unit_from_vbi_data_10bit(int line, uint8_t *src,
uint8_t *tgt)
+{
+ uint8_t y[720];
+ uint8_t *py = y;
+ uint8_t *pend = y + 720;
+ while (py < pend) {
+ *py++ = (src[1] >> 4) + ((src[2] & 15) << 4);
+ *py++ = (src[4] >> 2) + ((src[5] & 3 ) << 6);
+ *py++ = (src[6] >> 6) + ((src[7] & 63) << 2);
+ src += 8;
+ }
It would seem that the purpose of this code is to convert between the
Blackmagic 10-bit format (v210, according to the Blackmagic
documentation) and YUV420 (which is presumably easier than converting to
UYVY). Why not use a color converter for this? Looking through the
ffmpeg code base, I see that v210 is handled using a decoder/encoder and
not as a pixel format, which seems odd, but maybe there is some way to
take advantage of that support instead of doing it yourself. At the
very least, it seems reasonable to add a comment explaining what this
code is doing.
Also, I understand that this is being done because libzvbi doesn't
support the v210 pixel format (which would be useful to explain in a
comment), but I have to wonder if the conversion between the 10-bit v210
pixel format and the 8-bit YUV420 pixel format will result in a loss of
data that is relevant for teletext. If you've accommodated that
possibility, it would be good to describe in a comment.
+ return teletext_data_unit_from_vbi_data(line, y, tgt, VBI_PIXFMT_YUV420);
}
#endif
@@ -359,7 +378,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
//fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
#if CONFIG_LIBZVBI
- if (!no_video && ctx->teletext_lines && videoFrame->GetPixelFormat() ==
bmdFormat8BitYUV && videoFrame->GetWidth() == 720) {
+ if (!no_video && ctx->teletext_lines) {
IDeckLinkVideoFrameAncillary *vanc;
AVPacket txt_pkt;
uint8_t txt_buf0[1611]; // max 35 * 46 bytes decoded teletext
lines + 1 byte data_identifier
@@ -368,16 +387,21 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
if (videoFrame->GetAncillaryData(&vanc) == S_OK) {
int i;
int64_t line_mask = 1;
+ BMDPixelFormat vanc_format = vanc->GetPixelFormat();
txt_buf[0] = 0x10; // data_identifier - EBU_data
txt_buf++;
- for (i = 6; i < 336; i++, line_mask <<= 1) {
- uint8_t *buf;
- if ((ctx->teletext_lines & line_mask) &&
vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) {
- if (teletext_data_unit_from_vbi_data(i, buf, txt_buf)
>= 0)
- txt_buf += 46;
+ if (videoFrame->GetWidth() == 720 && (vanc_format ==
bmdFormat8BitYUV || vanc_format == bmdFormat10BitYUV)) {
According to the documentation in indevs.texi, the teletext support only
works for SD PAL. Yet, this check doesn't isolate things just to SD
PAL. I know that this issue was preexisting (although you have moved
the check to a different place in the code), but if you want to restrict
this code to SD PAL, then the best way is to check if the BMDDisplayMode
is bmdModePAL. If you want to base things on the width/height, you
could check for a width of 720 and a height of 576, although this could
indicate either bmdModePAL or bmdModePALp (although there may be no
Blackmagic devices that support bmdModePALp on input). Just checking
for a width of 720 includes NTSC, NTSCp, PAL, and PALp.
+ for (i = 6; i < 336; i++, line_mask <<= 1) {
+ uint8_t *buf;
+ if ((ctx->teletext_lines & line_mask) &&
vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) {
+ if (vanc_format == bmdFormat8BitYUV)
+ txt_buf = teletext_data_unit_from_vbi_data(i,
buf, txt_buf, VBI_PIXFMT_UYVY);
+ else
+ txt_buf =
teletext_data_unit_from_vbi_data_10bit(i, buf, txt_buf);
Since you already know if the format is 10-bit or 8-bit prior to
entering the for loop, putting the check here may result in an
unnecessary branch every loop iteration, depending on how the compiler
optimizes the code. So, for efficiency, it would make more sense to
move the if check to earlier, prior to executing the for loop, but this
will end up causing you to effectively duplicate the for loop code the
way it is currently written.
+ }
+ if (i == 22)
+ i = 317;
}
- if (i == 22)
- i = 317;
}
vanc->Release();
if (txt_buf - txt_buf0 > 1) {
Overall, looks good. I'll review the second patch in a bit.
Aaron Levinson
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
