[FFmpeg-devel] [PATCH 2/2] avdevice/decklink_dec: add support for receiving op47 teletext

Aaron Levinson alevinsn at aracnet.com
Mon Jul 3 19:34:11 EEST 2017


On 6/30/2017 5:05 PM, Marton Balint wrote:
> Signed-off-by: Marton Balint <cus at 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
> - at 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
> + at 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


More information about the ffmpeg-devel mailing list