On 6/30/2017 5:05 PM, Marton Balint wrote:
Signed-off-by: Marton Balint <c...@passwd.hu>
---
doc/indevs.texi | 18 ++++---
libavdevice/decklink_dec.cpp | 125 +++++++++++++++++++++++++++++++++++++++----
2 files changed, 125 insertions(+), 18 deletions(-)
diff --git a/doc/indevs.texi b/doc/indevs.texi
index 330617a7c9..2815248750 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -245,13 +245,17 @@ of uyvy422. Not all Blackmagic devices support this
option.
@item teletext_lines
If set to nonzero, an additional teletext stream will be captured from the
-vertical ancillary data. This option is a bitmask of the VBI lines checked,
-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. To use this
-option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
+vertical ancillary data. Both SD PAL and HD sources (OP47) are supported.
+
+This option is a bitmask of the SD VBI lines captured, specifically lines 6 to
Probably should be "SD PAL VBI lines" to make it clear that NTSC is not
supported.
+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.
+
+For SD sources ffmpeg needs to be compiled with @code{--enable-libzvbi}. For HD
+sources on older (pre-4K) DeckLink card models you have to capture in 10 bit
+mode.
Would be good to indicate that the bit mask is ignored for HD sources.
The documentation indicates that both SD PAL and HD sources are
supported, but an examination of the changes indicates that only some HD
sources are supported. Specifically, for HD sources to work, it expects
a width of 1920. This would cover both 1080i and 1080p, but it doesn't
cover 720p, with is also an HD video mode. My guess is that the code
has probably only been tested with 1080i as well, and in that case, it
would make sense to only specify 1080i in the documentation. Further,
since the original code only supports SD PAL, I would suspect that the
latest code has only been tested using "PAL" frame rates at 1080i, i.e.
1080i50. If it is unclear if the code will also work with 1080i59.94
and 1080i60, then it would be best to only support 1080i50.
Also, should have a comma after "For SD sources" and after "For HD sources".
@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 88af01be70..a4781dd072 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -35,6 +35,7 @@ extern "C" {
#include "libavutil/imgutils.h"
#include "libavutil/time.h"
#include "libavutil/mathematics.h"
+#include "libavutil/reverse.h"
#if CONFIG_LIBZVBI
#include <libzvbi.h>
#endif
@@ -43,7 +44,6 @@ extern "C" {
#include "decklink_common.h"
#include "decklink_dec.h"
-#if CONFIG_LIBZVBI
static uint8_t calc_parity_and_line_offset(int line)
{
uint8_t ret = (line < 313) << 5;
@@ -62,6 +62,7 @@ static void fill_data_unit_head(int line, uint8_t *tgt)
tgt[3] = 0xe4; // framing code
}
+#if CONFIG_LIBZVBI
static uint8_t* teletext_data_unit_from_vbi_data(int line, uint8_t *src,
uint8_t *tgt, vbi_pixfmt fmt)
{
vbi_bit_slicer slicer;
@@ -91,6 +92,94 @@ static uint8_t* teletext_data_unit_from_vbi_data_10bit(int
line, uint8_t *src, u
}
#endif
+static uint8_t* teletext_data_unit_from_op47_vbi_packet(int line, int *py, uint8_t *tgt)
+{
+ int i;
+
+ if (py[0] != 0x255 || py[1] != 0x255 || py[2] != 0x227)
+ return tgt;
+
+ fill_data_unit_head(line, tgt);
+
+ py += 3;
+ tgt += 4;
+
+ for (i = 0; i < 42; i++)
+ *tgt++ = ff_reverse[py[i] & 255];
+
+ return tgt;
+}
+
+static int linemask_matches(int line, int64_t mask)
+{
+ int shift = -1;
+ if (line >= 6 && line <= 22)
+ shift = line - 6;
+ if (line >= 318 && line <= 335)
+ shift = line - 318 + 17;
+ return shift >= 0 && ((1ULL << shift) & mask);
+}
+
+static uint8_t* teletext_data_unit_from_op47_data(int *py, int *pend, uint8_t
*tgt, int64_t wanted_lines)
+{
+ if (py < pend - 9) {
+ if (py[0] == 0x151 && py[1] == 0x115 && py[3] == 0x102) { //
identifier, identifier, format code for WST teletext
+ int *descriptors = py + 4;
+ int i;
+ py += 9;
+ for (i = 0; i < 5 && py < pend - 45; i++, py += 45) {
+ int line = (descriptors[i] & 31) + (!(descriptors[i] & 128)) *
313;
+ if (line && linemask_matches(line, wanted_lines))
+ tgt = teletext_data_unit_from_op47_vbi_packet(line, py,
tgt);
+ }
+ }
+ }
+ return tgt;
+}
+
+static uint8_t* teletext_data_unit_from_ancillary_packet(int *py, int *pend,
uint8_t *tgt, int64_t wanted_lines, int allow_multipacket)
+{
+ int did = py[0]; //
data id
+ int sdid = py[1]; //
secondary data id
+ int dc = py[2] & 255; //
data count
+ py += 3;
+ pend = FFMIN(pend, py + dc);
+ if (did == 0x143 && sdid == 0x102) { //
subtitle distribution packet
+ tgt = teletext_data_unit_from_op47_data(py, pend, tgt, wanted_lines);
+ } else if (allow_multipacket && did == 0x143 && sdid == 0x203) { //
VANC multipacket
+ py += 2; //
priority, line/field
+ while (py < pend - 3) {
+ tgt = teletext_data_unit_from_ancillary_packet(py, pend, tgt,
wanted_lines, 0);
+ py += 4 + (py[2] & 255); //
ndid, nsdid, ndc, line/field
+ }
+ }
+ return tgt;
+}
+
+static uint8_t* teletext_data_unit_from_vanc_data(uint8_t *src, uint8_t *tgt,
int64_t wanted_lines)
+{
+ int y[1920];
+ int *py = y;
+ int *pend = y + 1920;
+ while (py < pend) {
+ *py++ = (src[1] >> 2) + ((src[2] & 15) << 6);
+ *py++ = src[4] + ((src[5] & 3) << 8);
+ *py++ = (src[6] >> 4) + ((src[7] & 63) << 4);
+ src += 8;
+ }
My comments from the last review pertain to this code. Plus, now this
code is duplicated, so it would make sense to consolidate it to a new
function.
+ py = y;
+ while (py < pend - 6) {
+ if (py[0] == 0 && py[1] == 0x3ff && py[2] == 0x3ff) { //
ancilliary data flag
"ancilliary" -> "ancillary" in the comment
+ py += 3;
+ tgt = teletext_data_unit_from_ancillary_packet(py, pend, tgt,
wanted_lines, 0);
+ py += py[2] & 255;
+ } else {
+ py++;
+ }
+ }
+ return tgt;
+}
+
static void avpacket_queue_init(AVFormatContext *avctx, AVPacketQueue *q)
{
memset(q, 0, sizeof(AVPacketQueue));
@@ -377,11 +466,10 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
videoFrame->GetHeight();
//fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
-#if CONFIG_LIBZVBI
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
+ uint8_t txt_buf0[3531]; // 35 * 46 bytes decoded teletext lines +
1 byte data_identifier + 1920 bytes OP47 decode buffer
It is a little hard to follow this code, but it would seem that a max of
1611 or 1920 bytes would be needed (so 1920), not 1611 + 1920 bytes.
uint8_t *txt_buf = txt_buf0;
if (videoFrame->GetAncillaryData(&vanc) == S_OK) {
@@ -390,6 +478,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
BMDPixelFormat vanc_format = vanc->GetPixelFormat();
txt_buf[0] = 0x10; // data_identifier - EBU_data
txt_buf++;
+#if CONFIG_LIBZVBI
if (videoFrame->GetWidth() == 720 && (vanc_format ==
bmdFormat8BitYUV || vanc_format == bmdFormat10BitYUV)) {
for (i = 6; i < 336; i++, line_mask <<= 1) {
uint8_t *buf;
@@ -403,6 +492,20 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
i = 317;
}
}
+#endif
+ if (videoFrame->GetWidth() == 1920 && vanc_format ==
bmdFormat10BitYUV) {
As written, this would support all 1080p and 1080i video modes supported
by DeckLink. I would suspect that is not the desired behavior. See the
relevant comments from my review of the first patch.
+ for (i = 8; i < 584; i++) {
+ uint8_t *buf;
+ if (vanc->GetBufferForVerticalBlankingLine(i,
(void**)&buf) == S_OK)
+ txt_buf = teletext_data_unit_from_vanc_data(buf,
txt_buf, ctx->teletext_lines);
+ if (i == 20)
+ i = 569;
+ if (txt_buf - txt_buf0 > 1611) {
Since you added 1920 bytes to support OP47, it would seem that all 1920
bytes may be relevant. So, this above check may be wrong.
+ av_log(avctx, AV_LOG_ERROR, "Too many OP47 teletext
packets.\n");
+ break;
+ }
+ }
+ }
vanc->Release();
if (txt_buf - txt_buf0 > 1) {
int stuffing_units = (4 - ((45 + txt_buf - txt_buf0) /
46) % 4) % 4;
@@ -423,7 +526,6 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
}
}
}
-#endif
if (avpacket_queue_put(&ctx->queue, &pkt) < 0) {
++ctx->dropped;
@@ -522,13 +624,6 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
ctx->draw_bars = cctx->draw_bars;
cctx->ctx = ctx;
-#if !CONFIG_LIBZVBI
- if (ctx->teletext_lines) {
- av_log(avctx, AV_LOG_ERROR, "Libzvbi support is needed for capturing
teletext, please recompile FFmpeg.\n");
- return AVERROR(ENOSYS);
- }
-#endif
-
/* Check audio channel option for valid values: 2, 8 or 16 */
switch (cctx->audio_channels) {
case 2:
@@ -582,6 +677,14 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
}
}
+#if !CONFIG_LIBZVBI
+ if (ctx->teletext_lines && ctx->bmd_width == 720) {
+ av_log(avctx, AV_LOG_ERROR, "Libzvbi support is needed for capturing SD
teletext, please recompile FFmpeg.\n");
+ ret = AVERROR(ENOSYS);
+ goto error;
+ }
+#endif
+
/* Setup streams. */
st = avformat_new_stream(avctx, NULL);
if (!st) {
Aaron Levinson
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
