| Message ID | 20230309140808.5946-1-dheitmueller@ltnglobal.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] decklink: Add support for compressed AC-3 output over SDI | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
>+static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, >+uint8_t **outbuf, int *outsize) { This is very interesting in many other contexts. My current patch serie is about demuxing s337 (targeting dolby_e) from wav files, and it would be great to be able to re-mux back to s337 in output formats, be it decklink, wav or any broadcast format. So, maybe this belongs to s337m.c ? It could be later updated to support dolby_e. Nicolas
On Fri, Mar 10, 2023 at 2:44 AM Nicolas Gaullier <nicolas.gaullier@cji.paris> wrote: > > >+static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, > >+uint8_t **outbuf, int *outsize) { > > This is very interesting in many other contexts. > My current patch serie is about demuxing s337 (targeting dolby_e) from wav files, and it would be great > to be able to re-mux back to s337 in output formats, be it decklink, wav or any broadcast format. > So, maybe this belongs to s337m.c ? It could be later updated to support dolby_e. Hi Nicolas, I've been following your work on s337 for a while now, as I have corresponding patches to decklink capture and it might be better to reuse your s337 avformat module from within the decklink libavdevice (i.e. as a sub-demuxer). At this point I'm inclined to keep the code separate/private to decklink. It's limited to AC-3 and only produces packets which are s16 little endian. At some point I think it would be worthwhile to have some common code that supports 20/24 bit, both endians, and other codecs like Dolby-E, but my concern was at this point that would require a bunch of extra testing (much of which I don't have the streams or hardware to do), and I didn't want to lock us permanently into an ABI that I didn't know would meet our needs long term. Moving it from a static function to something that can be shared is a relatively simple operation, but I want to wait until we have a second use case prior to refactoring the code. Regards, Devin
>At this point I'm inclined to keep the code separate/private to decklink. It's limited to AC-3 and only produces packets which are >s16 little endian. At some point I think it would be worthwhile to have some common code that supports 20/24 bit, both endians, and other codecs like Dolby-E, but my concern was at this point that would require a bunch of extra testing (much of which I don't have the >streams or hardware to do), and I didn't want to lock us permanently into an ABI that I didn't know would meet our needs long term. > >Moving it from a static function to something that can be shared is a relatively simple operation, but I want to wait until we have a second use case prior to refactoring the code. Fully understand, yes you're right, thank you for the answer. Nicolas
On Thu, 9 Mar 2023, Devin Heitmueller wrote: > Extend the decklink output to include support for compressed AC-3, > encapsulated using the SMPTE ST 377:2015 standard. > > This functionality can be exercised by using the "copy" codec when > the input audio stream is AC-3. For example: > > ./ffmpeg -i ~/foo.ts -codec:a copy -f decklink 'UltraStudio Mini Monitor' > > Note that the default behavior continues to be to do PCM output, > which means without specifying the copy codec a stream containing > AC-3 will be decoded and downmixed to stereo audio before output. > > Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> > --- > libavdevice/decklink_enc.cpp | 97 ++++++++++++++++++++++++++++++------ > 1 file changed, 82 insertions(+), 15 deletions(-) > > diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp > index 8d423f6b6e..51a2bb4aad 100644 > --- a/libavdevice/decklink_enc.cpp > +++ b/libavdevice/decklink_enc.cpp > @@ -243,19 +243,32 @@ static int decklink_setup_audio(AVFormatContext *avctx, AVStream *st) > av_log(avctx, AV_LOG_ERROR, "Only one audio stream is supported!\n"); > return -1; > } > - if (c->sample_rate != 48000) { > - av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate!" > - " Only 48kHz is supported.\n"); > - return -1; > - } > - if (c->ch_layout.nb_channels != 2 && c->ch_layout.nb_channels != 8 && c->ch_layout.nb_channels != 16) { > - av_log(avctx, AV_LOG_ERROR, "Unsupported number of channels!" > - " Only 2, 8 or 16 channels are supported.\n"); > + > + if (c->codec_id == AV_CODEC_ID_AC3) { > + /* Regardless of the number of channels in the codec, we're only > + using 2 SDI audio channels at 48000Hz */ > + ctx->channels = 2; > + } else if (c->codec_id == AV_CODEC_ID_PCM_S16LE) { > + if (c->sample_rate != 48000) { > + av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate!" > + " Only 48kHz is supported.\n"); > + return -1; > + } > + if (c->ch_layout.nb_channels != 2 && c->ch_layout.nb_channels != 8 && c->ch_layout.nb_channels != 16) { > + av_log(avctx, AV_LOG_ERROR, "Unsupported number of channels!" > + " Only 2, 8 or 16 channels are supported.\n"); > + return -1; > + } > + ctx->channels = c->ch_layout.nb_channels; > + } else { > + av_log(avctx, AV_LOG_ERROR, "Unsupported codec specified!" > + " Only PCM_S16LE and AC-3 are supported.\n"); > return -1; > } > + > if (ctx->dlo->EnableAudioOutput(bmdAudioSampleRate48kHz, > bmdAudioSampleType16bitInteger, > - c->ch_layout.nb_channels, > + ctx->channels, > bmdAudioOutputStreamTimestamped) != S_OK) { > av_log(avctx, AV_LOG_ERROR, "Could not enable audio output!\n"); > return -1; > @@ -266,14 +279,49 @@ static int decklink_setup_audio(AVFormatContext *avctx, AVStream *st) > } > > /* 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. > > 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. > + > + /* Encapsulate AC3 syncframe into SMPTE 337 packet */ > + *outsize = pkt->size + 8; > + s337_payload = (uint8_t *) av_mallocz(*outsize); > + if (s337_payload == NULL) > + return AVERROR(ENOMEM); > + > + /* Construct SMPTE S337 Burst preamble */ > + s337_payload[0] = 0x72; /* Sync Word 1 */ > + s337_payload[1] = 0xf8; /* Sync Word 1 */ > + s337_payload[2] = 0x1f; /* Sync Word 1 */ > + s337_payload[3] = 0x4e; /* Sync Word 1 */ > + s337_payload[4] = 0x01; /* Burst Info, including data type (1=ac3) */ > + s337_payload[5] = 0x00; > + uint16_t bitcount = pkt->size * 8; > + s337_payload[6] = bitcount & 0xff; /* Length code */ > + s337_payload[7] = bitcount >> 8; /* Length code */ > + s337_payload_start = &s337_payload[8]; > + for (i = 0; i < pkt->size; i += 2) { > + s337_payload_start[0] = pkt->data[i+1]; > + s337_payload_start[1] = pkt->data[i]; > + s337_payload_start += 2; > + } > + > + *outbuf = s337_payload; > + return 0; > +} > + > av_cold int ff_decklink_write_trailer(AVFormatContext *avctx) > { > struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data; > @@ -531,21 +579,40 @@ static int decklink_write_audio_packet(AVFormatContext *avctx, AVPacket *pkt) > { > struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data; > struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx; > - int sample_count = pkt->size / (ctx->channels << 1); > + AVStream *st = avctx->streams[pkt->stream_index]; > + int sample_count; > uint32_t buffered; > + uint8_t *outbuf = NULL; > + int ret = 0; > > ctx->dlo->GetBufferedAudioSampleFrameCount(&buffered); > if (pkt->pts > 1 && !buffered) > av_log(avctx, AV_LOG_WARNING, "There's no buffered audio." > " Audio will misbehave!\n"); > > - 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. > + 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? > + } else { > + sample_count = pkt->size / (ctx->channels << 1); > + outbuf = pkt->data; > + } > + > + if (ctx->dlo->ScheduleAudioSamples(outbuf, sample_count, pkt->pts, > bmdAudioSampleRate48kHz, NULL) != S_OK) { > av_log(avctx, AV_LOG_ERROR, "Could not schedule audio samples.\n"); > - return AVERROR(EIO); > + ret = AVERROR(EIO); > } > > - return 0; > + if (st->codecpar->codec_id == AV_CODEC_ID_AC3) > + av_freep(&outbuf); > + > + return ret; > } Thanks, Marton
On Mon, Mar 13, 2023 at 7:38 PM Marton Balint <cus@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
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp index 8d423f6b6e..51a2bb4aad 100644 --- a/libavdevice/decklink_enc.cpp +++ b/libavdevice/decklink_enc.cpp @@ -243,19 +243,32 @@ static int decklink_setup_audio(AVFormatContext *avctx, AVStream *st) av_log(avctx, AV_LOG_ERROR, "Only one audio stream is supported!\n"); return -1; } - if (c->sample_rate != 48000) { - av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate!" - " Only 48kHz is supported.\n"); - return -1; - } - if (c->ch_layout.nb_channels != 2 && c->ch_layout.nb_channels != 8 && c->ch_layout.nb_channels != 16) { - av_log(avctx, AV_LOG_ERROR, "Unsupported number of channels!" - " Only 2, 8 or 16 channels are supported.\n"); + + if (c->codec_id == AV_CODEC_ID_AC3) { + /* Regardless of the number of channels in the codec, we're only + using 2 SDI audio channels at 48000Hz */ + ctx->channels = 2; + } else if (c->codec_id == AV_CODEC_ID_PCM_S16LE) { + if (c->sample_rate != 48000) { + av_log(avctx, AV_LOG_ERROR, "Unsupported sample rate!" + " Only 48kHz is supported.\n"); + return -1; + } + if (c->ch_layout.nb_channels != 2 && c->ch_layout.nb_channels != 8 && c->ch_layout.nb_channels != 16) { + av_log(avctx, AV_LOG_ERROR, "Unsupported number of channels!" + " Only 2, 8 or 16 channels are supported.\n"); + return -1; + } + ctx->channels = c->ch_layout.nb_channels; + } else { + av_log(avctx, AV_LOG_ERROR, "Unsupported codec specified!" + " Only PCM_S16LE and AC-3 are supported.\n"); return -1; } + if (ctx->dlo->EnableAudioOutput(bmdAudioSampleRate48kHz, bmdAudioSampleType16bitInteger, - c->ch_layout.nb_channels, + ctx->channels, bmdAudioOutputStreamTimestamped) != S_OK) { av_log(avctx, AV_LOG_ERROR, "Could not enable audio output!\n"); return -1; @@ -266,14 +279,49 @@ static int decklink_setup_audio(AVFormatContext *avctx, AVStream *st) } /* 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); 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); + + /* Encapsulate AC3 syncframe into SMPTE 337 packet */ + *outsize = pkt->size + 8; + s337_payload = (uint8_t *) av_mallocz(*outsize); + if (s337_payload == NULL) + return AVERROR(ENOMEM); + + /* Construct SMPTE S337 Burst preamble */ + s337_payload[0] = 0x72; /* Sync Word 1 */ + s337_payload[1] = 0xf8; /* Sync Word 1 */ + s337_payload[2] = 0x1f; /* Sync Word 1 */ + s337_payload[3] = 0x4e; /* Sync Word 1 */ + s337_payload[4] = 0x01; /* Burst Info, including data type (1=ac3) */ + s337_payload[5] = 0x00; + uint16_t bitcount = pkt->size * 8; + s337_payload[6] = bitcount & 0xff; /* Length code */ + s337_payload[7] = bitcount >> 8; /* Length code */ + s337_payload_start = &s337_payload[8]; + for (i = 0; i < pkt->size; i += 2) { + s337_payload_start[0] = pkt->data[i+1]; + s337_payload_start[1] = pkt->data[i]; + s337_payload_start += 2; + } + + *outbuf = s337_payload; + return 0; +} + av_cold int ff_decklink_write_trailer(AVFormatContext *avctx) { struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data; @@ -531,21 +579,40 @@ static int decklink_write_audio_packet(AVFormatContext *avctx, AVPacket *pkt) { struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data; struct decklink_ctx *ctx = (struct decklink_ctx *)cctx->ctx; - int sample_count = pkt->size / (ctx->channels << 1); + AVStream *st = avctx->streams[pkt->stream_index]; + int sample_count; uint32_t buffered; + uint8_t *outbuf = NULL; + int ret = 0; ctx->dlo->GetBufferedAudioSampleFrameCount(&buffered); if (pkt->pts > 1 && !buffered) av_log(avctx, AV_LOG_WARNING, "There's no buffered audio." " Audio will misbehave!\n"); - 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); + if (ret) + return ret; + sample_count = outbuf_size / 4; + } else { + sample_count = pkt->size / (ctx->channels << 1); + outbuf = pkt->data; + } + + if (ctx->dlo->ScheduleAudioSamples(outbuf, sample_count, pkt->pts, bmdAudioSampleRate48kHz, NULL) != S_OK) { av_log(avctx, AV_LOG_ERROR, "Could not schedule audio samples.\n"); - return AVERROR(EIO); + ret = AVERROR(EIO); } - return 0; + if (st->codecpar->codec_id == AV_CODEC_ID_AC3) + av_freep(&outbuf); + + return ret; } extern "C" {
Extend the decklink output to include support for compressed AC-3, encapsulated using the SMPTE ST 377:2015 standard. This functionality can be exercised by using the "copy" codec when the input audio stream is AC-3. For example: ./ffmpeg -i ~/foo.ts -codec:a copy -f decklink 'UltraStudio Mini Monitor' Note that the default behavior continues to be to do PCM output, which means without specifying the copy codec a stream containing AC-3 will be decoded and downmixed to stereo audio before output. Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> --- libavdevice/decklink_enc.cpp | 97 ++++++++++++++++++++++++++++++------ 1 file changed, 82 insertions(+), 15 deletions(-)