On Wed, 27 Sep 2017, Jeyapal, Karthick wrote:

Sorry for the late reply, the last was busy week. Here are some more
comments:

Thanks for your comments. Please find the updated patch attached with your 
comments incorporated.

Sorry, still not quite there... Here are some further comments:

From 9a572cb8b4ea599ee6bc1eedd6c7ca6868cfe0c6 Mon Sep 17 00:00:00 2001
From: Karthick J <kjeya...@akamai.com>
Date: Wed, 30 Aug 2017 11:43:16 +0530
Subject: [PATCH 3/4] avdevice/decklink_dec: Added Closed caption decode from
 VANC

Signed-off-by: Karthick J <kjeya...@akamai.com>
---
 libavcodec/avcodec.h         |   7 ++
 libavcodec/avpacket.c        |   1 +
 libavcodec/version.h         |   2 +-
 libavdevice/decklink_dec.cpp | 196 +++++++++++++++++++++++++++++++++++++++----
 4 files changed, 188 insertions(+), 18 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 513236a..d4a458f 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1600,6 +1600,13 @@ enum AVPacketSideDataType {
     AV_PKT_DATA_CONTENT_LIGHT_LEVEL,

     /**
+     * ATSC A53 Part 4 Closed Captions. This metadata should be associated with
+     * a video stream. A53 CC bitstream is stored as uint8_t in 
AVPacketSideData.data.
+     * The number of bytes of CC data is AVPacketSideData.size.
+     */
+    AV_PKT_DATA_A53_CC,

I forgot to mention that you should also add a note about this to 
doc/APIChanges, sorry.

+
+    /**
      * The number of side data elements (in fact a bit more than it).
      * This is not part of the public API/ABI in the sense that it may
      * change when new side data types are added.
diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index 5ce3228..a292b15 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -387,6 +387,7 @@ const char *av_packet_side_data_name(enum 
AVPacketSideDataType type)
     case AV_PKT_DATA_MASTERING_DISPLAY_METADATA: return "Mastering display 
metadata";
     case AV_PKT_DATA_CONTENT_LIGHT_LEVEL:        return "Content light level 
metadata";
     case AV_PKT_DATA_SPHERICAL:                  return "Spherical Mapping";
+    case AV_PKT_DATA_A53_CC:                     return "A53 Closed Captions";
     }
     return NULL;
 }
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 22cab7b..29cdb85 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -29,7 +29,7 @@

 #define LIBAVCODEC_VERSION_MAJOR  57
 #define LIBAVCODEC_VERSION_MINOR 104
-#define LIBAVCODEC_VERSION_MICRO 100
+#define LIBAVCODEC_VERSION_MICRO 101

 #define LIBAVCODEC_VERSION_INT  AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
                                                LIBAVCODEC_VERSION_MINOR, \
diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
index 38e6bd8..a24f563 100644
--- a/libavdevice/decklink_dec.cpp
+++ b/libavdevice/decklink_dec.cpp
@@ -1,6 +1,7 @@
 /*
  * Blackmagic DeckLink input
  * Copyright (c) 2013-2014 Luca Barbato, Deti Fliegl
+ * Copyright (c) 2014 Rafaël Carré
  * Copyright (c) 2017 Akamai Technologies, Inc.
  *
  * This file is part of FFmpeg.
@@ -105,6 +106,42 @@ static int get_vanc_line_idx(BMDDisplayMode mode)
     return i - 1;
 }

+static inline uint16_t parity (uint16_t x)
+{
+    uint16_t i;
+    for (i = 4 * sizeof (x); i > 0; i /= 2)
+        x ^= x >> i;
+    return x & 1;
+}
+
+static inline void clear_parity_bits(uint16_t *buf, int len) {
+    int i;
+    for (i = 0; i < len; i++)
+        buf[i] &= 0xff;
+}
+
+static int check_vanc_parity_checksum(uint16_t *buf, int len, uint16_t 
checksum) {
+    int i;
+    uint32_t vanc_sum;

uint16_t is enough, it does not matter if it overflows. Also don't forget to
initialize this to 0, the previous patch had that.

+    for (i = 3; i < len - 1; i++) {
+        uint16_t v = buf[i];
+        int np = v >> 8;
+        int p = parity(v & 0xff);
+        if ((!!p ^ !!(v & 0x100)) || (np != 1 && np != 2)) {
+            // Parity check failed
+            return -1;
+        }
+        vanc_sum += v;
+    }
+    vanc_sum &= 0x1ff;
+    vanc_sum |= ((~vanc_sum & 0x100) << 1);
+    if (checksum != vanc_sum) {
+        // Checksum verification failed
+        return -1;
+    }
+    return 0;
+}
+
 /* The 10-bit VANC data is packed in V210, we only need the luma component. */
 static void extract_luma_from_v210(uint16_t *dst, const uint8_t *src, int 
width)
 {
@@ -232,19 +269,148 @@ static uint8_t* 
teletext_data_unit_from_ancillary_packet(uint16_t *py, uint16_t
     return tgt;
 }

-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 i, len = (buf[5] & 0xff) + 6 + 1;
+    uint8_t cdp_sum, rate;
+    uint16_t hdr, ftr;
+    uint8_t *cc;
+    uint16_t *cdp = &buf[6]; // CDP follows
+    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;
+    }

-    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;
+    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;
+    }
+
+    cdp_sum = 0;
+    for (i = 0; i < len - 1; i++)
+        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;
+    }
+
+    rate = cdp[3];
+    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;
+    }
+
+    hdr = (cdp[5] << 8) | cdp[6];
+    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;
+    }
+
+    ftr = (cdp[len - 3] << 8) | cdp[len - 2];
+    if (ftr != hdr) {
+        av_log(avctx, AV_LOG_WARNING, "Header 0x%.4x != Footer 0x%.4x", hdr, 
ftr);
+        return NULL;
+    }
+
+    cc = (uint8_t *)av_malloc(cc_count * 3);
+    if (cc == NULL) {
+        av_log(avctx, AV_LOG_WARNING, "CC - av_malloc failed for cc_count = 
%d", cc_count);

You should add "\n" to the end of all these warnings.

+        return NULL;
+    }
+
+    for (size_t i = 0; i < cc_count; i++) {
+        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;
+
+    while (buf < max_buf - 6) {
+        int len;
+        uint16_t did = buf[3] & 0xFF;                                  // data 
id
+        uint16_t sdid = buf[4] & 0xFF;                                 // 
secondary data id
+        /* Check for VANC header */
+        if (buf[0] != 0 || buf[1] != 0x3ff || buf[2] != 0x3ff) {
+            return tgt;
+        }
+
+        len = (buf[5] & 0xff) + 6 + 1;
+        if (len > max_buf - buf) {
+            av_log(avctx, AV_LOG_WARNING, "Data Count (%d) > data left 
(%zu)\n",
+                    len, max_buf - buf);
+            return tgt;
+        }
+
+        if (did == 0x43 && (sdid == 0x02 || sdid == 0x03) && cctx->teletext_lines 
&&
+            width == 1920 && tgt_size >= 1920) {
+            if (!check_vanc_parity_checksum(buf, len, buf[len - 1])) {

The check is inverted. Use the common ffmpeg practice: if (function() < 0) that 
means error.

+                av_log(avctx, AV_LOG_WARNING, "VANC parity or checksum incorrect 
\n");

no need for extra space before \n.

+                goto skip_packet;
+            }
+            tgt = teletext_data_unit_from_ancillary_packet(buf + 3, buf + len, 
tgt, cctx->teletext_lines, 0);
+        } else if (did == 0x61 && sdid == 0x01) {
+            unsigned int data_len;
+            uint8_t *data;
+            if (!check_vanc_parity_checksum(buf, len, buf[len - 1])) {

The check is inverted. Use the common ffmpeg practice: if (function() < 0) that 
means error.

Also please test your patches with real CC data before posting them, I know it is tedious work with all the patch iterations, but I do the same with teletext.

+                av_log(avctx, AV_LOG_WARNING, "VANC parity or checksum incorrect 
\n");

no need for extra space before \n.

+                goto skip_packet;
+            }
+            clear_parity_bits(buf, len);
+            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);
+                if (pkt_cc)
+                    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);
         }
+skip_packet:
+        buf += len;
     }
+
     return tgt;
 }

@@ -536,7 +702,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 +715,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 +738,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, sizeof(txt_buf0) - 
(txt_buf - txt_buf0), &pkt);
                         }
                         if (i == vanc_line_numbers[idx].field0_vanc_end)
                             i = vanc_line_numbers[idx].field1_vanc_start - 1;
--
1.9.1


Fingers crossed for the next patch version :)

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to