On Tue, 5 Sep 2017, Jeyapal, Karthick wrote:
On 9/3/17, 8:19 PM, "Marton Balint" <c...@passwd.hu<mailto:c...@passwd.hu>>
wrote:
Why don't use you simply use uint16_t here as well? I think we typically
try to avoid unsigned as a type.
Done. Changed it to uint16_t.
This VANC parser does not seem right. VANC header can appear multiple
times, you seem to parse only the first DID. Also, you should simply
ignore unknown DID-s, ffmpeg will not be able to parse all types there
are, there is no need to warn.
Thanks for pointing it out. Have handled it now. Also changed the warning log
to debug log.
Please find the patch attached.
Here are some more comments for the code:
[..]
-static uint8_t* teletext_data_unit_from_vanc_data(uint16_t *py, uint8_t *tgt,
int64_t wanted_lines)
+uint8_t *vanc_to_cc(AVFormatContext *avctx, uint16_t *buf, size_t words,
+ unsigned &cc_count)
{
- uint16_t *pend = py + 1920;
+ size_t len = (buf[5] & 0xff) + 6 + 1;
- while (py < pend - 6) {
- if (py[0] == 0 && py[1] == 0x3ff && py[2] == 0x3ff) { //
ancillary data flag
- py += 3;
- tgt = teletext_data_unit_from_ancillary_packet(py, pend, tgt,
wanted_lines, 0);
- py += py[2] & 255;
+ /* CDP follows */
+ uint16_t *cdp = &buf[6];
Inline declarations are not allowed in ffmpeg.
+ if (cdp[0] != 0x96 || cdp[1] != 0x69) {
+ av_log(avctx, AV_LOG_WARNING, "Invalid CDP header 0x%.2x 0x%.2x",
cdp[0], cdp[1]);
+ return NULL;
+ }
+
+ len -= 7; // remove VANC header and checksum
+
+ if (cdp[2] != len) {
+ av_log(avctx, AV_LOG_WARNING, "CDP len %d != %zu", cdp[2], len);
+ return NULL;
+ }
+
+ uint8_t cdp_sum = 0;
inline declaration
+ for (size_t i = 0; i < len - 1; i++)
inline declaration
+ cdp_sum += cdp[i];
+ cdp_sum = cdp_sum ? 256 - cdp_sum : 0;
+ if (cdp[len - 1] != cdp_sum) {
+ av_log(avctx, AV_LOG_WARNING, "CDP checksum invalid 0x%.4x != 0x%.4x",
cdp_sum, cdp[len-1]);
+ return NULL;
+ }
+
+ uint8_t rate = cdp[3];
inline declaration
+ if (!(rate & 0x0f)) {
+ av_log(avctx, AV_LOG_WARNING, "CDP frame rate invalid (0x%.2x)", rate);
+ return NULL;
+ }
+ rate >>= 4;
+ if (rate > 8) {
+ av_log(avctx, AV_LOG_WARNING, "CDP frame rate invalid (0x%.2x)", rate);
+ return NULL;
+ }
+
+ if (!(cdp[4] & 0x43)) /* ccdata_present | caption_service_active |
reserved */ {
+ av_log(avctx, AV_LOG_WARNING, "CDP flags invalid (0x%.2x)", cdp[4]);
+ return NULL;
+ }
+
+ uint16_t hdr = (cdp[5] << 8) | cdp[6];
inline declaration
+ if (cdp[7] != 0x72) /* ccdata_id */ {
+ av_log(avctx, AV_LOG_WARNING, "Invalid ccdata_id 0x%.2x", cdp[7]);
+ return NULL;
+ }
+
+ cc_count = cdp[8];
+ if (!(cc_count & 0xe0)) {
+ av_log(avctx, AV_LOG_WARNING, "Invalid cc_count 0x%.2x", cc_count);
+ return NULL;
+ }
+
+ cc_count &= 0x1f;
+ if ((len - 13) < cc_count * 3) {
+ av_log(avctx, AV_LOG_WARNING, "Invalid cc_count %d (> %zu)", cc_count
* 3, len - 13);
+ return NULL;
+ }
+
+ if (cdp[len - 4] != 0x74) /* footer id */ {
+ av_log(avctx, AV_LOG_WARNING, "Invalid footer id 0x%.2x", cdp[len-4]);
+ return NULL;
+ }
+
+ uint16_t ftr = (cdp[len - 3] << 8) | cdp[len - 2];
inline declaration
+ if (ftr != hdr) {
+ av_log(avctx, AV_LOG_WARNING, "Header 0x%.4x != Footer 0x%.4x", hdr,
ftr);
+ return NULL;
+ }
+
+ uint8_t *cc = (uint8_t *)av_malloc(cc_count * 3);
inline declaration
malloc missing error check
+
+ for (size_t i = 0; i < cc_count; i++) {
inline declaration
+ cc[3*i + 0] = cdp[9 + 3*i+0] /* & 3 */;
+ cc[3*i + 1] = cdp[9 + 3*i+1];
+ cc[3*i + 2] = cdp[9 + 3*i+2];
+ }
+
+ cc_count *= 3;
+ return cc;
+}
+
+uint8_t *get_metadata(AVFormatContext *avctx, uint16_t *buf, size_t width,
+ uint8_t *tgt, size_t tgt_size, AVPacket *pkt)
+{
+ decklink_cctx *cctx = (struct decklink_cctx *) avctx->priv_data;
+ uint16_t *max_buf = buf + width;
+
+ static const uint8_t vanc_header[6] = { 0x00, 0x00, 0xff, 0x03, 0xff, 0x03
};
+ while (buf < max_buf - 6) {
+ uint16_t did = buf[3] & 0xFF; // data
id
+ uint16_t sdid = buf[4] & 0xFF; //
secondary data id
+ /* Check for VANC header */
+ if (memcmp(vanc_header, buf, 3 * 2)) {
Maybe it's personal preference, but I find the (buf[0] == 0 && buf[1] ==
0x3ff && buf[2] == 0x3ff) better. Also, maybe performance wise it might be
faster
then calling memcmp each time, although I am not sure.
Also since we are going to parse vanc in every capture, I wonder if we
should simply take into account the general requirement that vanc packets
should start immediately and they should be contiguous. As far as I know,
with compliant sources, we can simply return here instead of continuing
the loop, and that definitely improves performance.
+ buf++;
+ continue;
+ }
+
+ size_t len = (buf[5] & 0xff) + 6 + 1;
inline declaration
+ if (len > max_buf - buf) {
+ av_log(avctx, AV_LOG_WARNING, "Data Count (%zu) > data left
(%zu)\n",
+ len, max_buf - buf);
+ return tgt;
+ }
+
+ uint16_t vanc_sum = 0;
inline declaration
+ for (size_t i = 3; i < len - 1; i++) {
inline declaration
+ uint16_t v = buf[i];
+ int np = v >> 8;
+ int p = parity(v & 0xff);
+ if ((!!p ^ !!(v & 0x100)) || (np != 1 && np != 2)) {
+ av_log(avctx, AV_LOG_WARNING, "Parity incorrect for word
%zu\n", i);
In case of a parity error, you should only skip the current VANC packet, and
continue parsing
+ return tgt;
+ }
+ vanc_sum += v;
+ vanc_sum &= 0x1ff;
+ buf[i] &= 0xff;
+ }
+
+ vanc_sum |= ((~vanc_sum & 0x100) << 1);
+ if (buf[len - 1] != vanc_sum) {
+ av_log(avctx, AV_LOG_WARNING,
+ "VANC checksum incorrect: 0x%.4x != 0x%.4x\n", vanc_sum,
+ buf[len - 1]);
You can also continue with the next VANC packet here instead of returning
+ return tgt;
+ }
+
+ if (did == 0x43 && (sdid == 0x02 || sdid == 0x03) && cctx->teletext_lines
&&
+ width == 1920 && tgt_size >= 1920) {
+ tgt = teletext_data_unit_from_ancillary_packet(buf, buf + 1920, tgt,
cctx->teletext_lines, 0);
this should be
tgt = teletext_data_unit_from_ancillary_packet(buf, buf + len, tgt,
cctx->teletext_lines, 1);
+ } else if (did == 0x61 && sdid == 0x01) {
+ unsigned int data_len;
+ uint8_t *data = vanc_to_cc(avctx, buf, width, data_len);
+ if (data) {
+ uint8_t *pkt_cc = av_packet_new_side_data(pkt,
AV_PKT_DATA_A53_CC, data_len);
missing error check
+ memcpy(pkt_cc, data, data_len);
+ av_free(data);
+ }
} else {
- py++;
+ av_log(avctx, AV_LOG_DEBUG, "Unknown meta data DID = 0x%.2x SDID =
0x%.2x\n",
+ did, sdid);
}
+ buf += len;
}
+
return tgt;
}
@@ -536,7 +678,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
videoFrame->GetHeight();
//fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
- if (!no_video && ctx->teletext_lines) {
+ if (!no_video) {
IDeckLinkVideoFrameAncillary *vanc;
AVPacket txt_pkt;
uint8_t txt_buf0[3531]; // 35 * 46 bytes decoded teletext lines +
1 byte data_identifier + 1920 bytes OP47 decode buffer
@@ -549,7 +691,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
txt_buf[0] = 0x10; // data_identifier - EBU_data
txt_buf++;
#if CONFIG_LIBZVBI
- if (ctx->bmd_mode == bmdModePAL && (vanc_format ==
bmdFormat8BitYUV || vanc_format == bmdFormat10BitYUV)) {
+ if (ctx->bmd_mode == bmdModePAL && ctx->teletext_lines &&
+ (vanc_format == bmdFormat8BitYUV || vanc_format ==
bmdFormat10BitYUV)) {
av_assert0(videoFrame->GetWidth() == 720);
for (i = 6; i < 336; i++, line_mask <<= 1) {
uint8_t *buf;
@@ -571,13 +714,8 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
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 (videoFrame->GetWidth() == 1920) {
- txt_buf =
teletext_data_unit_from_vanc_data(luma_vanc, txt_buf, ctx->teletext_lines);
- if (txt_buf - txt_buf0 > 1611) { // ensure
we still have at least 1920 bytes free in the buffer
- av_log(avctx, AV_LOG_ERROR, "Too many OP47
teletext packets.\n");
- break;
- }
- }
+ txt_buf = get_metadata(avctx, luma_vanc,
videoFrame->GetWidth(),
+ txt_buf, 3531 - (txt_buf -
txt_buf0), &pkt);
sizeof(txt_buf0) instead of 3531
}
if (i == vanc_line_numbers[idx].field0_vanc_end)
i = vanc_line_numbers[idx].field1_vanc_start;
--
1.9.1
Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
