[FFmpeg-devel] [PATCH 3/4] avdevice/decklink_dec: Added Closed caption decode from VANC

Marton Balint cus at passwd.hu
Thu Sep 7 22:31:18 EEST 2017


On Tue, 5 Sep 2017, Jeyapal, Karthick wrote:

>
>
>> On 9/3/17, 8:19 PM, "Marton Balint" <cus at passwd.hu<mailto:cus at 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


More information about the ffmpeg-devel mailing list