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".
next prev parent 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