[FFmpeg-devel] [PATCH 1/8] libavdevice/decklink: Add support for EIA-708 output over SDI

Devin Heitmueller dheitmueller at ltnglobal.com
Fri Dec 29 23:00:00 EET 2017


Hi Carl,

Thanks for your feedback.  Comments inline>

> On Dec 29, 2017, at 3:41 PM, Carl Eugen Hoyos <ceffmpeg at gmail.com> wrote:
> 
> 2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmueller at ltnglobal.com>:
> 
>> +    side_data = av_frame_get_side_data(pic, AV_FRAME_DATA_A53_CC);
>> +    if (side_data && side_data->size) {
>> +        uint8_t *buf = av_packet_new_side_data(pkt, AV_PKT_DATA_A53_CC, side_data->size);
>> +        if (buf)
>> +            memcpy(buf, side_data->data, side_data->size);
>> +        else
>> +            return AVERROR(ENOMEM);
> 
> 
> Maybe you disagree but the following is slightly simpler imo:
> 
>  if (!buf)
>    return AVERROR();
>  memcpy(buf, data, size);

I don’t feel strongly one way or the other.  If changing it gets it merged, then I will resubmit.

> 
> [...]
> 
>> +#if CONFIG_LIBKLVANC
> 
> I tried to voice this before and I assume there is no solution
> but this is a large optional code block depending on an
> external library inside an optional feature depending on
> another - not super-common - external library: This will not
> get much testing, not even for compilation.

It’s a bit of a chicken-and-egg problem.  People who are most likely to use SDI are in the broadcast industry, and they typically can’t use ffmpeg in production due to all this missing functionality.  My hope is that as we fill those holes there will be increased use of the ffmpeg decklink module.

Separating the functionality into a separate library (libklvanc) was intentional, as it allows us to use the same VANC processing code across multiple open source products (we have versions of VLC and OBE which incorporate the library and have been deployed in production for months).  It also allows us to share the same code across multiple card manufacturers, although there aren’t many to choose from at this point.

I don’t doubt it won’t see much testing at this point given the relatively small number of users making use of decklink in ffmpeg.  However that can describe a bunch of features in ffmpeg, and I still think there is a lot of benefit to getting this stuff upstreamed for those who do want to do SDI capture/output.

I’m not discounting your concerns - just there isn’t much I can do about them but continue to try to improve the quality of the code until it’s reliable enough for wider adoption.  :-)

Devin


More information about the ffmpeg-devel mailing list