[FFmpeg-devel] [PATCH] decklink: Add support for compressed AC-3 output over SDI

[Devin Heitmueller]

On Mon, Mar 13, 2023 at 7:38 PM Marton Balint <cus at passwd.hu> wrote:
> >     /* The device expects the sample rate to be fixed. */
> > -    avpriv_set_pts_info(st, 64, 1, c->sample_rate);
> > -    ctx->channels = c->ch_layout.nb_channels;
> > +    avpriv_set_pts_info(st, 64, 1, bmdAudioSampleRate48kHz);
>
> I'd rather not use a BMD constant for non-BMD function. Make this 48000 or
> create a non-BMD 48000 constant and use it here and when checking the
> sample rate earlier.

Ok, good point.  Will change to 48000.

>
> >
> >     ctx->audio = 1;
> >
> >     return 0;
> > }
> >
> > +static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, uint8_t **outbuf, int *outsize)
> > +{
> > +    uint8_t *s337_payload;
> > +    uint8_t *s337_payload_start;
> > +    int i;
> > +
> > +    if (codec_id != AV_CODEC_ID_AC3)
> > +        return AVERROR(EINVAL);
>
> Use the PutByteContext for this function.

Ok.

> > -    if (ctx->dlo->ScheduleAudioSamples(pkt->data, sample_count, pkt->pts,
> > +    if (st->codecpar->codec_id == AV_CODEC_ID_AC3) {
> > +        /* Encapsulate AC3 syncframe into SMPTE 337 packet */
> > +        int outbuf_size;
> > +        ret = create_s337_payload(pkt, st->codecpar->codec_id,
> > +                                  &outbuf, &outbuf_size);
>
> Can't you create a buffer on the stack instead of using a dynamic
> memory allocation for each frame? I guess the S337 packet should never
> exceed the uncompressed size, but AC3 might have even more strict frame
> size limits.

I generally avoid putting things on the stack which are of
indeterminate size.  Also, I've got a patch coming which has to make a
copy of all audio data (to interleave different audio streams together
prior to output).  In that case heap usage is unavoidable and thus I
didn't think it was worthwhile to allocate a 1500+ byte stack buffer.

> > +        if (ret)
> > +            return ret;
> > +        sample_count = outbuf_size / 4;
>
> How is it ensured that enough raw audio data is provided for Decklink? Or
> we provide "sparse" audio data to the decklink API, and decklink will
> pad the audio with silence based on the packet timestamps?

This patch relies on the hardware automatically padding the audio
(which does work correctly).  I have a subsequent patch which always
inserts empty audio if there are gaps (as part of the work to
interleave multiple audio streams).

I will submit a revised version of the patch incorporating your
comments.  Thanks for reviewing!

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller at ltnglobal.com