Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Devin Heitmueller <devin.heitmueller@ltnglobal.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v3] decklink: Add support for compressed AC-3 output over SDI
Date: Wed, 29 Mar 2023 09:13:01 -0400
Message-ID: <CAHGibzHWN7nkELPybthKqNwMaBZkTBeuO5p7sx0UxE+PAj-SNA@mail.gmail.com> (raw)
In-Reply-To: <aac8b047-f3f8-db9c-3f3d-8345b9d14bab@passwd.hu>

Hi Marton,

Thanks for reviewing!

On Tue, Mar 28, 2023 at 3:37 PM Marton Balint <cus@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@ltnglobal.com
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-03-29 13:13 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-27 16:47 Devin Heitmueller
2023-03-28 19:36 ` Marton Balint
2023-03-29 13:13   ` Devin Heitmueller [this message]
  -- strict thread matches above, loose matches on Subject: below --
2023-03-27 16:08 Devin Heitmueller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHGibzHWN7nkELPybthKqNwMaBZkTBeuO5p7sx0UxE+PAj-SNA@mail.gmail.com \
    --to=devin.heitmueller@ltnglobal.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git