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

[Devin Heitmueller]

Hi Marton,

Thanks for reviewing!

On Tue, Mar 28, 2023 at 3:37 PM Marton Balint <cus at passwd.hu> wrote:
> > +    if (codec_id != AV_CODEC_ID_AC3)
> > +        return AVERROR(EINVAL);
>
> The codec check might be overkill here, after all you are calling this
> only from code which already checked this. Or later you extend this
> somehow?

S337 encapsulation supports a number of different codecs, but I wanted to
be clear to any developers who looked at the code that this version
currently only supports AC-3.  That said, you're right that you would never
fail this check without further code changes, so I can just change this to
a comment in case somebody comes along and wants to add support for other
codecs.

> > +
> > +    /* Sanity check:  According to SMPTE ST 340:2015 Sec 4.1, the AC-3
sync frame will
> > +       exactly match the 1536 samples of baseband (PCM) audio that it
represents.  */
> > +    if (pkt->size > 1536)
> > +        return AVERROR(EINVAL);
> > +
> > +    /* Encapsulate AC3 syncframe into SMPTE 337 packet */
> > +    s337_payload = (uint8_t *) av_malloc(payload_size);
> > +    if (s337_payload == NULL)
> > +        return AVERROR(ENOMEM);
> > +    bytestream2_init_writer(&pb, s337_payload, payload_size);
> > +    bytestream2_put_le16u(&pb, 0xf872); /* Sync word 1 */
> > +    bytestream2_put_le16u(&pb, 0x4e1f); /* Sync word 1 */
> > +    bytestream2_put_le16u(&pb, 0x0001); /* Burst Info, including data
type (1=ac3) */
> > +    bytestream2_put_le16u(&pb, bitcount); /* Length code */
> > +    for (int i = 0; i < pkt->size; i += 2)
> > +        bytestream2_put_le16u(&pb, (pkt->data[i] << 8) |
pkt->data[i+1]);
>
> This can overread/ovewrite 1 byte if pkt->size is odd.

Fair point.  I'll adjust the loop to write one fewer and handle the
remainder if present.

Devin

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