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

Add support for the ability to capture all audio pairs available
to the capture hardware.  Each pair is exposed as a different audio
stream, which matches up with the most common use cases for the
broadcast space (i.e. where there is one stereo pair per audio
language).

To support the existing use case where multi-channel audio can be
captured (i.e. 7.1), we introduced a new configuration option, which
defaults to the existing behavior.
---
 libavdevice/decklink_common.cpp |   9 +++
 libavdevice/decklink_common.h   |   8 ++-
 libavdevice/decklink_common_c.h |   6 ++
 libavdevice/decklink_dec.cpp    | 134 +++++++++++++++++++++++++++++++---------
 libavdevice/decklink_dec_c.c    |   3 +
 5 files changed, 130 insertions(+), 30 deletions(-)

2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:

> +        for (int i = 0; i < ctx->max_audio_channels / 2; i++) {
> +            st = avformat_new_stream(avctx, NULL);
> +            if (!st) {
> +                av_log(avctx, AV_LOG_ERROR, "Cannot add stream %d\n", i);
> +                ret = AVERROR(ENOMEM);
> +                goto error;
> +            }
> +            st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
> +            st->codecpar->codec_id    = ctx->audio_depth == 32 ?
> AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
> +            st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
> +            st->codecpar->channels    = 2;
> +            avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
> +            ctx->audio_st[i] = st;
> +            ctx->num_audio_streams++;
> +        }

I would have expected that the channel_layout is set to STEREO in
this case, is that not always true?

Carl Eugen

Hi Carl,

> On Dec 29, 2017, at 3:55 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 
> 2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:
> 
>> +        for (int i = 0; i < ctx->max_audio_channels / 2; i++) {
>> +            st = avformat_new_stream(avctx, NULL);
>> +            if (!st) {
>> +                av_log(avctx, AV_LOG_ERROR, "Cannot add stream %d\n", i);
>> +                ret = AVERROR(ENOMEM);
>> +                goto error;
>> +            }
>> +            st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
>> +            st->codecpar->codec_id    = ctx->audio_depth == 32 ?
>> AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
>> +            st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
>> +            st->codecpar->channels    = 2;
>> +            avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
>> +            ctx->audio_st[i] = st;
>> +            ctx->num_audio_streams++;
>> +        }
> 
> I would have expected that the channel_layout is set to STEREO in
> this case, is that not always true?

I’m not sure I understand your comment.  Is there some channel layout property of the codec parameters I should be setting?

For the moment, it’s true that we’re only supporting capturing stereo pairs.  But coming down the pipe is support for compressed audio over SDI pairs, as well as more complex layouts which involve discrete 5.1 or 7.1 channels.  This patch is a stepping stone to that (I’ve designed it with those use cases in mind, even though I haven’t implemented them yet).

For example, it’s not uncommon to have a series of SDI pairs such as the following:

SDI channels 1-6 contain discrete 5.1 audio as PCM
SDI channels 7-8 contain a stereo PCM pair with a second language
SDI channels 9-10 contain a compressed 5.1 AC-3 stream.

This patch doesn’t let you do the above, but it’s working in that direction.

Devin

2017-12-29 22:14 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:
> Hi Carl,
>
>> On Dec 29, 2017, at 3:55 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>
>> 2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:
>>
>>> +        for (int i = 0; i < ctx->max_audio_channels / 2; i++) {
>>> +            st = avformat_new_stream(avctx, NULL);
>>> +            if (!st) {
>>> +                av_log(avctx, AV_LOG_ERROR, "Cannot add stream %d\n", i);
>>> +                ret = AVERROR(ENOMEM);
>>> +                goto error;
>>> +            }
>>> +            st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
>>> +            st->codecpar->codec_id    = ctx->audio_depth == 32 ?
>>> AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
>>> +            st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
>>> +            st->codecpar->channels    = 2;
>>> +            avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
>>> +            ctx->audio_st[i] = st;
>>> +            ctx->num_audio_streams++;
>>> +        }
>>
>> I would have expected that the channel_layout is set to STEREO in
>> this case, is that not always true?
>
> I’m not sure I understand your comment.  Is there some channel layout
> property of the codec parameters I should be setting?

Yes, AVCodecParameters->channel_layout.

Carl Eugen

> On Dec 29, 2017, at 4:17 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
> 
> 2017-12-29 22:14 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:
>> Hi Carl,
>> 
>>> On Dec 29, 2017, at 3:55 PM, Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote:
>>> 
>>> 2017-12-29 19:12 GMT+01:00 Devin Heitmueller <dheitmueller@ltnglobal.com>:
>>> 
>>>> +        for (int i = 0; i < ctx->max_audio_channels / 2; i++) {
>>>> +            st = avformat_new_stream(avctx, NULL);
>>>> +            if (!st) {
>>>> +                av_log(avctx, AV_LOG_ERROR, "Cannot add stream %d\n", i);
>>>> +                ret = AVERROR(ENOMEM);
>>>> +                goto error;
>>>> +            }
>>>> +            st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
>>>> +            st->codecpar->codec_id    = ctx->audio_depth == 32 ?
>>>> AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
>>>> +            st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
>>>> +            st->codecpar->channels    = 2;
>>>> +            avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
>>>> +            ctx->audio_st[i] = st;
>>>> +            ctx->num_audio_streams++;
>>>> +        }
>>> 
>>> I would have expected that the channel_layout is set to STEREO in
>>> this case, is that not always true?
>> 
>> I’m not sure I understand your comment.  Is there some channel layout
>> property of the codec parameters I should be setting?
> 
> Yes, AVCodecParameters->channel_layout.

Ah, ok.  I will fix that and resubmit in the next series.

Thanks,

Devin

On 12/29/2017 10:12 AM, Devin Heitmueller wrote:
> Add support for the ability to capture all audio pairs available
> to the capture hardware.  Each pair is exposed as a different audio
> stream, which matches up with the most common use cases for the
> broadcast space (i.e. where there is one stereo pair per audio
> language).
> 
> To support the existing use case where multi-channel audio can be
> captured (i.e. 7.1), we introduced a new configuration option, which
> defaults to the existing behavior.
> ---
>   libavdevice/decklink_common.cpp |   9 +++
>   libavdevice/decklink_common.h   |   8 ++-
>   libavdevice/decklink_common_c.h |   6 ++
>   libavdevice/decklink_dec.cpp    | 134 +++++++++++++++++++++++++++++++---------
>   libavdevice/decklink_dec_c.c    |   3 +
>   5 files changed, 130 insertions(+), 30 deletions(-)
> 
> diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
> index ba091dadea..91a626221d 100644
> --- a/libavdevice/decklink_common.cpp
> +++ b/libavdevice/decklink_common.cpp
> @@ -480,5 +480,14 @@ int ff_decklink_init_device(AVFormatContext *avctx, const char* name)
>           return AVERROR_EXTERNAL;
>       }
>   
> +    if (ctx->attr->GetInt(BMDDeckLinkMaximumAudioChannels, &ctx->max_audio_channels) != S_OK) {
> +        av_log(avctx, AV_LOG_WARNING, "Could not determine number of audio channels\n");
> +        ctx->max_audio_channels = 0;
> +    }
> +    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".

> +        ctx->max_audio_channels = DECKLINK_MAX_AUDIO_CHANNELS;
> +    }
> +
>       return 0;
>   }
> diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
> index 143bbb951a..06b241029e 100644
> --- a/libavdevice/decklink_common.h
> +++ b/libavdevice/decklink_common.h
> @@ -37,6 +37,10 @@
>   #define DECKLINK_BOOL bool
>   #endif
>   
> +/* Maximum number of channels possible across variants of Blackmagic cards.
> +   Actual number for any particular model of card may be lower */
> +#define DECKLINK_MAX_AUDIO_CHANNELS 32
> +
>   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.

>   
>       /* Capture buffer queue */
>       AVPacketQueue queue;
> @@ -85,7 +90,8 @@ struct decklink_ctx {
>       int64_t last_pts;
>       unsigned long frameCount;
>       unsigned int dropped;
> -    AVStream *audio_st;
> +    AVStream *audio_st[DECKLINK_MAX_AUDIO_CHANNELS];
> +    int num_audio_streams;
>       AVStream *video_st;
>       AVStream *teletext_st;
>       uint16_t cdp_sequence_num;
> diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
> index 368ac259e4..02011ed53b 100644
> --- a/libavdevice/decklink_common_c.h
> +++ b/libavdevice/decklink_common_c.h
> @@ -30,6 +30,11 @@ typedef enum DecklinkPtsSource {
>       PTS_SRC_WALLCLOCK = 4,
>   } DecklinkPtsSource;
>   
> +typedef enum DecklinkAudioMode {
> +    AUDIO_MODE_DISCRETE = 0,
> +    AUDIO_MODE_PAIRS = 1,
> +} DecklinkAudioMode;
> >   struct decklink_cctx {
>       const AVClass *cclass;
>   
> @@ -42,6 +47,7 @@ struct decklink_cctx {
>       double preroll;
>       int v210;
>       int audio_channels;
> +    int audio_mode;
>       int audio_depth;
>       int duplex_mode;
>       DecklinkPtsSource audio_pts_source;
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index 94dae26003..8d4070eecd 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -627,9 +627,54 @@ static int64_t get_pkt_pts(IDeckLinkVideoInputFrame *videoFrame,
>       return pts;
>   }
>   
> +static int setup_audio(AVFormatContext *avctx)
> +{
> +    struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
> +    struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx;
> +    AVStream *st;
> +    int ret = 0;
> +
> +    if (cctx->audio_mode == AUDIO_MODE_DISCRETE) {
> +        st = avformat_new_stream(avctx, NULL);
> +        if (!st) {
> +            av_log(avctx, AV_LOG_ERROR, "Cannot add stream\n");
> +            ret = AVERROR(ENOMEM);
> +            goto error;
> +        }
> +        st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
> +        st->codecpar->codec_id    = ctx->audio_depth == 32 ? AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
> +        st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
> +        st->codecpar->channels    = cctx->audio_channels;
> +        avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
> +        ctx->audio_st[0] = st;
> +        ctx->num_audio_streams++;
> +    } else {
> +        for (int i = 0; i < ctx->max_audio_channels / 2; i++) {

Technically, you declared max_audio_channels as an int64_t, but i is 
declared as an int.

> +            st = avformat_new_stream(avctx, NULL);
> +            if (!st) {
> +                av_log(avctx, AV_LOG_ERROR, "Cannot add stream %d\n", i);
> +                ret = AVERROR(ENOMEM);
> +                goto error;
> +            }
> +            st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
> +            st->codecpar->codec_id    = ctx->audio_depth == 32 ? AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
> +            st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
> +            st->codecpar->channels    = 2;
> +            avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
> +            ctx->audio_st[i] = st;
> +            ctx->num_audio_streams++;
> +        }
> +        cctx->audio_channels = ctx->max_audio_channels;
> +    }
> +
> +error:
> +    return ret;
> +}
> +
>   HRESULT decklink_input_callback::VideoInputFrameArrived(
>       IDeckLinkVideoInputFrame *videoFrame, IDeckLinkAudioInputPacket *audioFrame)
>   {
> +    decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data;
>       void *frameBytes;
>       void *audioFrameBytes;
>       BMDTimeValue frameTime;
> @@ -777,24 +822,57 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
>   
>       // Handle Audio Frame
>       if (audioFrame) {
> -        AVPacket pkt;
> -        BMDTimeValue audio_pts;
> -        av_init_packet(&pkt);
> -
> -        //hack among hacks
> -        pkt.size = audioFrame->GetSampleFrameCount() * ctx->audio_st->codecpar->channels * (ctx->audio_depth / 8);
>           audioFrame->GetBytes(&audioFrameBytes);
> -        audioFrame->GetPacketTime(&audio_pts, ctx->audio_st->time_base.den);
> -        pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, ctx->audio_pts_source, ctx->audio_st->time_base, &initial_audio_pts);
> -        pkt.dts = pkt.pts;
>   
> -        //fprintf(stderr,"Audio Frame size %d ts %d\n", pkt.size, pkt.pts);
> -        pkt.flags       |= AV_PKT_FLAG_KEY;
> -        pkt.stream_index = ctx->audio_st->index;
> -        pkt.data         = (uint8_t *)audioFrameBytes;
> +        if (cctx->audio_mode == AUDIO_MODE_DISCRETE) {
> +            AVPacket pkt;
> +            BMDTimeValue audio_pts;
> +            av_init_packet(&pkt);
>   
> -        if (avpacket_queue_put(&ctx->queue, &pkt) < 0) {
> -            ++ctx->dropped;
> +            //hack among hacks
> +            pkt.size = audioFrame->GetSampleFrameCount() * ctx->audio_st[0]->codecpar->channels * (ctx->audio_depth / 8);
> +            audioFrame->GetBytes(&audioFrameBytes);
> +            audioFrame->GetPacketTime(&audio_pts, ctx->audio_st[0]->time_base.den);
> +            pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, ctx->audio_pts_source, ctx->audio_st[0]->time_base, &initial_audio_pts);
> +            pkt.dts = pkt.pts;
> +
> +            pkt.flags       |= AV_PKT_FLAG_KEY;
> +            pkt.stream_index = ctx->audio_st[0]->index;
> +            pkt.data         = (uint8_t *)audioFrameBytes;
> +
> +            if (avpacket_queue_put(&ctx->queue, &pkt) < 0) {
> +                ++ctx->dropped;
> +            }
> +        } else {
> +            /* Need to deinterleave audio */
> +            int audio_offset = 0;
> +            int audio_stride = cctx->audio_channels * ctx->audio_depth / 8;
> +            for (int i = 0; i < ctx->num_audio_streams; i++) {
> +                int sample_size = ctx->audio_st[i]->codecpar->channels *
> +                                  ctx->audio_st[i]->codecpar->bits_per_coded_sample / 8;
> +                AVPacket pkt;
> +                int ret = av_new_packet(&pkt, audioFrame->GetSampleFrameCount() * sample_size);
> +                if (ret != 0)
> +                    continue;
> +
> +                pkt.pts = get_pkt_pts(videoFrame, audioFrame, wallclock, ctx->audio_pts_source,
> +                                      ctx->audio_st[i]->time_base, &initial_audio_pts);
> +                pkt.dts          = pkt.pts;
> +                pkt.flags       |= AV_PKT_FLAG_KEY;
> +                pkt.stream_index = ctx->audio_st[i]->index;
> +
> +                uint8_t *audio_in = ((uint8_t *) audioFrameBytes) + audio_offset;
> +                for (int x = 0; x < pkt.size; x += sample_size) {
> +                    memcpy(&pkt.data[x], audio_in, sample_size);
> +                    audio_in += audio_stride;
> +                }
> +
> +                if (avpacket_queue_put(&ctx->queue, &pkt) < 0)
> +                    ++ctx->dropped;
> +
> +                av_packet_unref(&pkt);
> +                audio_offset += sample_size;
> +            }
>           }
>       }
>   
> @@ -999,18 +1077,7 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>   #endif
>   
>       /* Setup streams. */
> -    st = avformat_new_stream(avctx, NULL);
> -    if (!st) {
> -        av_log(avctx, AV_LOG_ERROR, "Cannot add stream\n");
> -        ret = AVERROR(ENOMEM);
> -        goto error;
> -    }
> -    st->codecpar->codec_type  = AVMEDIA_TYPE_AUDIO;
> -    st->codecpar->codec_id    = cctx->audio_depth == 32 ? AV_CODEC_ID_PCM_S32LE : AV_CODEC_ID_PCM_S16LE;
> -    st->codecpar->sample_rate = bmdAudioSampleRate48kHz;
> -    st->codecpar->channels    = cctx->audio_channels;
> -    avpriv_set_pts_info(st, 64, 1, 1000000);  /* 64 bits pts in us */
> -    ctx->audio_st=st;
> +    setup_audio(avctx);
>   
>       st = avformat_new_stream(avctx, NULL);
>       if (!st) {
> @@ -1096,8 +1163,17 @@ av_cold int ff_decklink_read_header(AVFormatContext *avctx)
>           ctx->teletext_st = st;
>       }
>   
> -    av_log(avctx, AV_LOG_VERBOSE, "Using %d input audio channels\n", ctx->audio_st->codecpar->channels);
> -    result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz, cctx->audio_depth == 32 ? bmdAudioSampleType32bitInteger : bmdAudioSampleType16bitInteger, ctx->audio_st->codecpar->channels);
> +    if (cctx->audio_mode == AUDIO_MODE_DISCRETE) {
> +        av_log(avctx, AV_LOG_VERBOSE, "Using %d input audio channels\n", ctx->audio_st[0]->codecpar->channels);
> +        result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz,
> +                                            ctx->audio_depth == 32 ? bmdAudioSampleType32bitInteger : bmdAudioSampleType16bitInteger,
> +                                            ctx->audio_st[0]->codecpar->channels);
> +    } else {
> +        av_log(avctx, AV_LOG_VERBOSE, "Using %d input audio channels\n", (int)ctx->max_audio_channels);
> +        result = ctx->dli->EnableAudioInput(bmdAudioSampleRate48kHz,
> +                                            ctx->audio_depth == 32 ? bmdAudioSampleType32bitInteger : bmdAudioSampleType16bitInteger,
> +                                            ctx->max_audio_channels);
> +    }
>   
>       if (result != S_OK) {
>           av_log(avctx, AV_LOG_ERROR, "Cannot enable audio input\n");
> diff --git a/libavdevice/decklink_dec_c.c b/libavdevice/decklink_dec_c.c
> index 1c6d826945..d3d8c848cf 100644
> --- a/libavdevice/decklink_dec_c.c
> +++ b/libavdevice/decklink_dec_c.c
> @@ -44,6 +44,9 @@ static const AVOption options[] = {
>       { "standard",     NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7fff9fffeLL}, 0, 0,    DEC, "teletext_lines"},
>       { "all",          NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = 0x7ffffffffLL}, 0, 0,    DEC, "teletext_lines"},
>       { "channels",     "number of audio channels", OFFSET(audio_channels), AV_OPT_TYPE_INT , { .i64 = 2   }, 2, 16, DEC },
> +    { "audio_mode",   "audio mode",               OFFSET(audio_mode),     AV_OPT_TYPE_INT,   { .i64 = AUDIO_MODE_DISCRETE}, 0, 1,    DEC, "audio_mode"},
> +    { "discrete",     NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = AUDIO_MODE_DISCRETE}, 0, 0,    DEC, "audio_mode"},
> +    { "pairs",        NULL,                                           0,  AV_OPT_TYPE_CONST, { .i64 = AUDIO_MODE_PAIRS}, 0, 0,    DEC, "audio_mode"},
>       { "duplex_mode",  "duplex mode",              OFFSET(duplex_mode),    AV_OPT_TYPE_INT,   { .i64 = 0}, 0, 2,    DEC, "duplex_mode"},
>       { "unset",         NULL,                                          0,  AV_OPT_TYPE_CONST, { .i64 = 0}, 0, 0,    DEC, "duplex_mode"},
>       { "half",          NULL,                                          0,  AV_OPT_TYPE_CONST, { .i64 = 1}, 0, 0,    DEC, "duplex_mode"},
> 

Aaron Levinson

Am 29.12.2017 um 19:12 schrieb Devin Heitmueller:
> To support the existing use case where multi-channel audio can be
> captured (i.e. 7.1)

Just to be clear, the current use case is NOT to capture multi-channel
audio like 7.1. It's just to capture all of the mono SDI channels into
one FFmpeg-internal audio stream and FFmpeg calls 8 channels "7.1".

As you already noticed, the audio mapping is far from standardized and
has to be adapted anyway in most use cases... having said that, I find
the option value "discrete" misleading. I would expect every channel to
be in a separate stream when reading that, but I'm not a native english
speaker. What about e.g. "bundled"?

Last but not least, I cannot find an update to the documentation in this
patch ;)

Regards
Matthias

>> +    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

Hello Matthias,

Thanks for the feedback.  Comments inline:

> On Jan 2, 2018, at 4:52 AM, Matthias Hunstock <atze@fem.tu-ilmenau.de> wrote:
> 
> Am 29.12.2017 um 19:12 schrieb Devin Heitmueller:
>> To support the existing use case where multi-channel audio can be
>> captured (i.e. 7.1)
> 
> Just to be clear, the current use case is NOT to capture multi-channel
> audio like 7.1. It's just to capture all of the mono SDI channels into
> one FFmpeg-internal audio stream and FFmpeg calls 8 channels "7.1”.

Right.  The existing capture module can capture 2, 8, or 16 mono channels IIRC, and they’re all bundled as a single stream.  The underlying channel map is unspecified and has to be tweaked through a filter if needed.
> 
> As you already noticed, the audio mapping is far from standardized and
> has to be adapted anyway in most use cases... having said that, I find
> the option value "discrete" misleading. I would expect every channel to
> be in a separate stream when reading that, but I'm not a native english
> speaker. What about e.g. "bundled”?

No objection here.  The term “discrete” is just what the broadcast industry uses when sending uncompressed multi-channel audio over SDI (e.g. when dealing with 5.1 or 7.1).  That said, even in the “pairs” model I’m introducing it’s entirely possible that some of those pairs will be components to a 5.1, so I have no objection to using the vague term “bundled” since it boils down to all the possible audio channels available from the card.

> 
> Last but not least, I cannot find an update to the documentation in this
> patch ;)

Yup, that is indeed needed.  Will include in next patch.

Devin