[FFmpeg-devel] [PATCH 6/8] Add suppoort for using libklvanc from within decklink capture module

[Devin Heitmueller]

Hi Aaron,

> On Dec 30, 2017, at 2:34 AM, Aaron Levinson <alevinsn_dev at levland.net> wrote:
> 
> Patch title:  "suppoort" -> "support", also "decklink" -> “DeckLink"

Ok.

>>        ff_decklink_cleanup(avctx);
>>      avpacket_queue_end(&ctx->queue);
>> +    klvanc_context_destroy(ctx->vanc_ctx);
> 
> Shouldn't this last line be wrapped in #if CONFIG_LIBKLVANC?  I suggest building this on Linux with and without libklvanc, and I also suggest building it (and ideally testing it) on Windows without libklvanc as well.  DeckLink is also supported on OS/X.

Yup, that was a mistake I made preparing the latest patch.  The #ifdef guard got left out.

I do typically test-compile on both Linux and OS X both with and without the library present, but that process got skipped this time around (which was an error on my part).

I have confirmed though that was the only build error when the library isn’t present.

> 
>>        av_freep(&cctx->ctx);
>>  @@ -1193,6 +1315,17 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>>        avpacket_queue_init (avctx, &ctx->queue);
>>  +#if CONFIG_LIBKLVANC
>> +    if (klvanc_context_create(&ctx->vanc_ctx) < 0) {
>> +        av_log(avctx, AV_LOG_ERROR, "Cannot create VANC library context\n");
>> +        ret = AVERROR(ENOMEM);
> 
> Perhaps appropriate to use the return value of klvanc_context_create() as input to AVERROR().

I think it’s usually bad practice to blindly return what a third party library returns.  Usually the error should be converted into some ffmpeg specific error code which the caller might actually know what to do with.  In this case I just return ENOMEM since that’s the only actual failure case possible in the call.

Devin