[FFmpeg-devel] [PATCH 3/8] decklink: Introduce support for capture of multiple audio streams

Devin Heitmueller dheitmueller at ltnglobal.com
Fri Jan 5 22:12:37 EET 2018


>> +    if (ctx->max_audio_channels > DECKLINK_MAX_AUDIO_CHANNELS) {
>> +        av_log(avctx, AV_LOG_WARNING, "Decklink card reported support for more channels than ffmpeg supports\n");
> 
> "Decklink" -> "DeckLink", "ffmpeg" -> "FFmpeg".  Also, I think it is preferable to not state "FFmpeg" in this log message, as technically this is in libavdevice.  If, say, libav were to merge your changes into their codebase, then this particular log message would make that annoying.  Could be something as simple as "Max audio channels for DeckLink reduced from %d to %d.\n”.

Ok, no objection

>>  class decklink_output_callback;
>>  class decklink_input_callback;
>>  @@ -71,6 +75,7 @@ struct decklink_ctx {
>>      int bmd_height;
>>      int bmd_field_dominance;
>>      int supports_vanc;
>> +    int64_t max_audio_channels;
> 
> Rationale for using an int64_t here when an int would likely be sufficient?  I understand that GetInt() takes an int64_t as input, but you could use a temporary int64_t with GetInt() and store the value in a int max_audio_channels.

I was just trying to avoid an intermediate variable and a cast.  Figured the added code wasn’t worth the trouble just to save eight bytes on a structure that there will likely only ever be one instance of.  No objection to doing what you’ve proposed though.

Devin


More information about the ffmpeg-devel mailing list