[FFmpeg-devel] [PATCH 1/2] avdevice/decklink_dec: add support for decoding teletext from 10bit ancillary data

Aaron Levinson alevinsn at aracnet.com
Mon Jul 3 18:51:57 EEST 2017


On 6/30/2017 5:05 PM, Marton Balint wrote:
> This also add supports for 4K DeckLink cards because they always output the
> ancillary data in 10-bit.
> 
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
>   doc/indevs.texi              |  4 ++--
>   libavdevice/decklink_dec.cpp | 56 +++++++++++++++++++++++++++++++-------------
>   2 files changed, 42 insertions(+), 18 deletions(-)
> 
> diff --git a/doc/indevs.texi b/doc/indevs.texi
> index 51c304f3ec..330617a7c9 100644
> --- a/doc/indevs.texi
> +++ b/doc/indevs.texi
> @@ -250,8 +250,8 @@ 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 in 8 bit mode.
> -To use this option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
> +receivers. Capturing teletext only works for SD PAL sources. To use this
> +option, ffmpeg needs to be compiled with @code{--enable-libzvbi}.
>   
>   @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 39974e3ff4..88af01be70 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -54,21 +54,40 @@ static uint8_t calc_parity_and_line_offset(int line)
>       return ret;
>   }
>   
> -int teletext_data_unit_from_vbi_data(int line, uint8_t *src, uint8_t *tgt)
> +static void fill_data_unit_head(int line, uint8_t *tgt)
> +{
> +    tgt[0] = 0x02; // data_unit_id
> +    tgt[1] = 0x2c; // data_unit_length
> +    tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
> +    tgt[3] = 0xe4; // framing code
> +}
> +
> +static uint8_t* teletext_data_unit_from_vbi_data(int line, uint8_t *src, uint8_t *tgt, vbi_pixfmt fmt)
>   {
>       vbi_bit_slicer slicer;
>   
> -    vbi_bit_slicer_init(&slicer, 720, 13500000, 6937500, 6937500, 0x00aaaae4, 0xffff, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, VBI_PIXFMT_UYVY);
> +    vbi_bit_slicer_init(&slicer, 720, 13500000, 6937500, 6937500, 0x00aaaae4, 0xffff, 18, 6, 42 * 8, VBI_MODULATION_NRZ_MSB, fmt);
>   
>       if (vbi_bit_slice(&slicer, src, tgt + 4) == FALSE)
> -        return -1;
> +        return tgt;
>   
> -    tgt[0] = 0x02; // data_unit_id
> -    tgt[1] = 0x2c; // data_unit_length
> -    tgt[2] = calc_parity_and_line_offset(line); // field_parity, line_offset
> -    tgt[3] = 0xe4; // framing code
> +    fill_data_unit_head(line, tgt);
>   
> -    return 0;
> +    return tgt + 46;
> +}
> +
> +static uint8_t* teletext_data_unit_from_vbi_data_10bit(int line, uint8_t *src, uint8_t *tgt)
> +{
> +    uint8_t y[720];
> +    uint8_t *py = y;
> +    uint8_t *pend = y + 720;
> +    while (py < pend) {
> +        *py++ = (src[1] >> 4) + ((src[2] & 15) << 4);
> +        *py++ = (src[4] >> 2) + ((src[5] & 3 ) << 6);
> +        *py++ = (src[6] >> 6) + ((src[7] & 63) << 2);
> +        src += 8;
> +    }

It would seem that the purpose of this code is to convert between the 
Blackmagic 10-bit format (v210, according to the Blackmagic 
documentation) and YUV420 (which is presumably easier than converting to 
UYVY).  Why not use a color converter for this?  Looking through the 
ffmpeg code base, I see that v210 is handled using a decoder/encoder and 
not as a pixel format, which seems odd, but maybe there is some way to 
take advantage of that support instead of doing it yourself.  At the 
very least, it seems reasonable to add a comment explaining what this 
code is doing.

Also, I understand that this is being done because libzvbi doesn't 
support the v210 pixel format (which would be useful to explain in a 
comment), but I have to wonder if the conversion between the 10-bit v210 
pixel format and the 8-bit YUV420 pixel format will result in a loss of 
data that is relevant for teletext.  If you've accommodated that 
possibility, it would be good to describe in a comment.

> +    return teletext_data_unit_from_vbi_data(line, y, tgt, VBI_PIXFMT_YUV420);
>   }
>   #endif
>   
> @@ -359,7 +378,7 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>           //fprintf(stderr,"Video Frame size %d ts %d\n", pkt.size, pkt.pts);
>   
>   #if CONFIG_LIBZVBI
> -        if (!no_video && ctx->teletext_lines && videoFrame->GetPixelFormat() == bmdFormat8BitYUV && videoFrame->GetWidth() == 720) {
> +        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
> @@ -368,16 +387,21 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>               if (videoFrame->GetAncillaryData(&vanc) == S_OK) {
>                   int i;
>                   int64_t line_mask = 1;
> +                BMDPixelFormat vanc_format = vanc->GetPixelFormat();
>                   txt_buf[0] = 0x10;    // data_identifier - EBU_data
>                   txt_buf++;
> -                for (i = 6; i < 336; i++, line_mask <<= 1) {
> -                    uint8_t *buf;
> -                    if ((ctx->teletext_lines & line_mask) && vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) {
> -                        if (teletext_data_unit_from_vbi_data(i, buf, txt_buf) >= 0)
> -                            txt_buf += 46;
> +                if (videoFrame->GetWidth() == 720 && (vanc_format == bmdFormat8BitYUV || vanc_format == bmdFormat10BitYUV)) {

According to the documentation in indevs.texi, the teletext support only 
works for SD PAL.  Yet, this check doesn't isolate things just to SD 
PAL.  I know that this issue was preexisting (although you have moved 
the check to a different place in the code), but if you want to restrict 
this code to SD PAL, then the best way is to check if the BMDDisplayMode 
is bmdModePAL.  If you want to base things on the width/height, you 
could check for a width of 720 and a height of 576, although this could 
indicate either bmdModePAL or bmdModePALp (although there may be no 
Blackmagic devices that support bmdModePALp on input).  Just checking 
for a width of 720 includes NTSC, NTSCp, PAL, and PALp.

> +                    for (i = 6; i < 336; i++, line_mask <<= 1) {
> +                        uint8_t *buf;
> +                        if ((ctx->teletext_lines & line_mask) && vanc->GetBufferForVerticalBlankingLine(i, (void**)&buf) == S_OK) {
> +                            if (vanc_format == bmdFormat8BitYUV)
> +                                txt_buf = teletext_data_unit_from_vbi_data(i, buf, txt_buf, VBI_PIXFMT_UYVY);
> +                            else
> +                                txt_buf = teletext_data_unit_from_vbi_data_10bit(i, buf, txt_buf);

Since you already know if the format is 10-bit or 8-bit prior to 
entering the for loop, putting the check here may result in an 
unnecessary branch every loop iteration, depending on how the compiler 
optimizes the code.  So, for efficiency, it would make more sense to 
move the if check to earlier, prior to executing the for loop, but this 
will end up causing you to effectively duplicate the for loop code the 
way it is currently written.

> +                        }
> +                        if (i == 22)
> +                            i = 317;
>                       }
> -                    if (i == 22)
> -                        i = 317;
>                   }
>                   vanc->Release();
>                   if (txt_buf - txt_buf0 > 1) {
> 

Overall, looks good.  I'll review the second patch in a bit.

Aaron Levinson


More information about the ffmpeg-devel mailing list