* [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h @ 2023-03-17 15:02 Devin Heitmueller 2023-03-17 15:02 ` [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI Devin Heitmueller 2023-03-25 4:47 ` [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Andreas Rheinhardt 0 siblings, 2 replies; 9+ messages in thread From: Devin Heitmueller @ 2023-03-17 15:02 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Devin Heitmueller When including the header in decklink_enc.cpp it would be fed through the C++ compiler rather than the C compiler, which has more strict warnings when comparing signed/unsigned values. Make the local variables unsigned to match the arguments they are being passed for those functions. Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> --- libavcodec/bytestream.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h index d0033f14f3..67080604b9 100644 --- a/libavcodec/bytestream.h +++ b/libavcodec/bytestream.h @@ -180,7 +180,7 @@ static av_always_inline void bytestream2_skipu(GetByteContext *g, static av_always_inline void bytestream2_skip_p(PutByteContext *p, unsigned int size) { - int size2; + unsigned int size2; if (p->eof) return; size2 = FFMIN(p->buffer_end - p->buffer, size); @@ -268,7 +268,7 @@ static av_always_inline unsigned int bytestream2_get_buffer(GetByteContext *g, uint8_t *dst, unsigned int size) { - int size2 = FFMIN(g->buffer_end - g->buffer, size); + unsigned int size2 = FFMIN(g->buffer_end - g->buffer, size); memcpy(dst, g->buffer, size2); g->buffer += size2; return size2; @@ -287,7 +287,7 @@ static av_always_inline unsigned int bytestream2_put_buffer(PutByteContext *p, const uint8_t *src, unsigned int size) { - int size2; + unsigned int size2; if (p->eof) return 0; size2 = FFMIN(p->buffer_end - p->buffer, size); @@ -311,7 +311,7 @@ static av_always_inline void bytestream2_set_buffer(PutByteContext *p, const uint8_t c, unsigned int size) { - int size2; + unsigned int size2; if (p->eof) return; size2 = FFMIN(p->buffer_end - p->buffer, size); @@ -348,7 +348,7 @@ static av_always_inline unsigned int bytestream2_copy_buffer(PutByteContext *p, GetByteContext *g, unsigned int size) { - int size2; + unsigned int size2; if (p->eof) return 0; -- 2.35.1.655.ga68dfadae5 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI 2023-03-17 15:02 [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Devin Heitmueller @ 2023-03-17 15:02 ` Devin Heitmueller 2023-03-24 21:07 ` Marton Balint 2023-03-25 4:47 ` [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Andreas Rheinhardt 1 sibling, 1 reply; 9+ messages in thread From: Devin Heitmueller @ 2023-03-17 15:02 UTC (permalink / raw) To: ffmpeg-devel; +Cc: Devin Heitmueller 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. Thanks to Marton Balint for providing feedback. Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> --- libavdevice/decklink_enc.cpp | 90 ++++++++++++++++++++++++++++++------ 1 file changed, 75 insertions(+), 15 deletions(-) diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp index 8d423f6b6e..8d80f00247 100644 --- a/libavdevice/decklink_enc.cpp +++ b/libavdevice/decklink_enc.cpp @@ -32,6 +32,7 @@ extern "C" { extern "C" { #include "libavformat/avformat.h" +#include "libavcodec/bytestream.h" #include "libavutil/internal.h" #include "libavutil/imgutils.h" #include "avdevice.h" @@ -243,19 +244,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 +280,41 @@ 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, 48000); ctx->audio = 1; return 0; } +static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, uint8_t **outbuf, int *outsize) +{ + int payload_size = pkt->size + 8; + uint16_t bitcount = pkt->size * 8; + uint8_t *s337_payload; + PutByteContext pb; + int i; + + if (codec_id != AV_CODEC_ID_AC3) + return AVERROR(EINVAL); + + /* Encapsulate AC3 syncframe into SMPTE 337 packet */ + s337_payload = (uint8_t *) av_mallocz(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 (i = 0; i < pkt->size; i += 2) + bytestream2_put_le16u(&pb, (pkt->data[i] << 8) | pkt->data[i+1]); + + *outsize = payload_size; + *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 +572,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" { -- 2.35.1.655.ga68dfadae5 _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI 2023-03-17 15:02 ` [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI Devin Heitmueller @ 2023-03-24 21:07 ` Marton Balint 2023-03-27 16:08 ` Devin Heitmueller 0 siblings, 1 reply; 9+ messages in thread From: Marton Balint @ 2023-03-24 21:07 UTC (permalink / raw) To: FFmpeg development discussions and patches On Fri, 17 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. > > Thanks to Marton Balint for providing feedback. > > Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> > --- > libavdevice/decklink_enc.cpp | 90 ++++++++++++++++++++++++++++++------ > 1 file changed, 75 insertions(+), 15 deletions(-) > > diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp > index 8d423f6b6e..8d80f00247 100644 > --- a/libavdevice/decklink_enc.cpp > +++ b/libavdevice/decklink_enc.cpp > @@ -32,6 +32,7 @@ extern "C" { > > extern "C" { > #include "libavformat/avformat.h" > +#include "libavcodec/bytestream.h" > #include "libavutil/internal.h" > #include "libavutil/imgutils.h" > #include "avdevice.h" > @@ -243,19 +244,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 +280,41 @@ 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, 48000); > > ctx->audio = 1; > > return 0; > } > > +static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, uint8_t **outbuf, int *outsize) > +{ > + int payload_size = pkt->size + 8; > + uint16_t bitcount = pkt->size * 8; > + uint8_t *s337_payload; > + PutByteContext pb; > + int i; > + > + if (codec_id != AV_CODEC_ID_AC3) > + return AVERROR(EINVAL); Maybe some sanity check here for pkt->size upper limit to avoid overflows? > + > + /* Encapsulate AC3 syncframe into SMPTE 337 packet */ > + s337_payload = (uint8_t *) av_mallocz(payload_size); Why not simply av_malloc? > + 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 (i = 0; i < pkt->size; i += 2) for (int i = > + bytestream2_put_le16u(&pb, (pkt->data[i] << 8) | pkt->data[i+1]); > + > + *outsize = payload_size; > + *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 +572,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) if (ret < 0) is preferred > + 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; > } > Thanks, Marton _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI 2023-03-24 21:07 ` Marton Balint @ 2023-03-27 16:08 ` Devin Heitmueller 0 siblings, 0 replies; 9+ messages in thread From: Devin Heitmueller @ 2023-03-27 16:08 UTC (permalink / raw) To: FFmpeg development discussions and patches On Fri, Mar 24, 2023 at 5:07 PM Marton Balint <cus@passwd.hu> wrote: > > > > On Fri, 17 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. > > > > Thanks to Marton Balint for providing feedback. > > > > Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> > > --- > > libavdevice/decklink_enc.cpp | 90 ++++++++++++++++++++++++++++++------ > > 1 file changed, 75 insertions(+), 15 deletions(-) > > > > diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp > > index 8d423f6b6e..8d80f00247 100644 > > --- a/libavdevice/decklink_enc.cpp > > +++ b/libavdevice/decklink_enc.cpp > > @@ -32,6 +32,7 @@ extern "C" { > > > > extern "C" { > > #include "libavformat/avformat.h" > > +#include "libavcodec/bytestream.h" > > #include "libavutil/internal.h" > > #include "libavutil/imgutils.h" > > #include "avdevice.h" > > @@ -243,19 +244,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 +280,41 @@ 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, 48000); > > > > ctx->audio = 1; > > > > return 0; > > } > > > > +static int create_s337_payload(AVPacket *pkt, enum AVCodecID codec_id, uint8_t **outbuf, int *outsize) > > +{ > > + int payload_size = pkt->size + 8; > > + uint16_t bitcount = pkt->size * 8; > > + uint8_t *s337_payload; > > + PutByteContext pb; > > + int i; > > + > > + if (codec_id != AV_CODEC_ID_AC3) > > + return AVERROR(EINVAL); > > Maybe some sanity check here for pkt->size upper limit to avoid overflows? > > > + > > + /* Encapsulate AC3 syncframe into SMPTE 337 packet */ > > + s337_payload = (uint8_t *) av_mallocz(payload_size); > > Why not simply av_malloc? > > > + 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 (i = 0; i < pkt->size; i += 2) > > for (int i = > > > + bytestream2_put_le16u(&pb, (pkt->data[i] << 8) | pkt->data[i+1]); > > + > > + *outsize = payload_size; > > + *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 +572,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) > > if (ret < 0) is preferred > > > + 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; > > } > > > > Thanks, > Marton Thanks for your feedback. A revised patch reflecting your changes will be sent to the mailing list shortly. 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h 2023-03-17 15:02 [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Devin Heitmueller 2023-03-17 15:02 ` [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI Devin Heitmueller @ 2023-03-25 4:47 ` Andreas Rheinhardt 2023-03-25 17:09 ` Marton Balint 1 sibling, 1 reply; 9+ messages in thread From: Andreas Rheinhardt @ 2023-03-25 4:47 UTC (permalink / raw) To: ffmpeg-devel Devin Heitmueller: > When including the header in decklink_enc.cpp it would be fed > through the C++ compiler rather than the C compiler, which has > more strict warnings when comparing signed/unsigned values. > > Make the local variables unsigned to match the arguments they are > being passed for those functions. > > Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> > --- > libavcodec/bytestream.h | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h > index d0033f14f3..67080604b9 100644 > --- a/libavcodec/bytestream.h > +++ b/libavcodec/bytestream.h > @@ -180,7 +180,7 @@ static av_always_inline void bytestream2_skipu(GetByteContext *g, > static av_always_inline void bytestream2_skip_p(PutByteContext *p, > unsigned int size) > { > - int size2; > + unsigned int size2; > if (p->eof) > return; > size2 = FFMIN(p->buffer_end - p->buffer, size); > @@ -268,7 +268,7 @@ static av_always_inline unsigned int bytestream2_get_buffer(GetByteContext *g, > uint8_t *dst, > unsigned int size) > { > - int size2 = FFMIN(g->buffer_end - g->buffer, size); > + unsigned int size2 = FFMIN(g->buffer_end - g->buffer, size); > memcpy(dst, g->buffer, size2); > g->buffer += size2; > return size2; > @@ -287,7 +287,7 @@ static av_always_inline unsigned int bytestream2_put_buffer(PutByteContext *p, > const uint8_t *src, > unsigned int size) > { > - int size2; > + unsigned int size2; > if (p->eof) > return 0; > size2 = FFMIN(p->buffer_end - p->buffer, size); > @@ -311,7 +311,7 @@ static av_always_inline void bytestream2_set_buffer(PutByteContext *p, > const uint8_t c, > unsigned int size) > { > - int size2; > + unsigned int size2; > if (p->eof) > return; > size2 = FFMIN(p->buffer_end - p->buffer, size); > @@ -348,7 +348,7 @@ static av_always_inline unsigned int bytestream2_copy_buffer(PutByteContext *p, > GetByteContext *g, > unsigned int size) > { > - int size2; > + unsigned int size2; > > if (p->eof) > return 0; The bytestream APIs are allowed to overread if the buffer is padded and the user manages this himself. So you are not allowed to presume that g->buffer_end - g->buffer is positive. - Andreas _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h 2023-03-25 4:47 ` [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Andreas Rheinhardt @ 2023-03-25 17:09 ` Marton Balint 2023-03-27 13:12 ` Devin Heitmueller 2023-03-27 13:26 ` Andreas Rheinhardt 0 siblings, 2 replies; 9+ messages in thread From: Marton Balint @ 2023-03-25 17:09 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sat, 25 Mar 2023, Andreas Rheinhardt wrote: > Devin Heitmueller: >> When including the header in decklink_enc.cpp it would be fed >> through the C++ compiler rather than the C compiler, which has >> more strict warnings when comparing signed/unsigned values. >> >> Make the local variables unsigned to match the arguments they are >> being passed for those functions. >> >> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> >> --- >> libavcodec/bytestream.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h >> index d0033f14f3..67080604b9 100644 >> --- a/libavcodec/bytestream.h >> +++ b/libavcodec/bytestream.h >> @@ -180,7 +180,7 @@ static av_always_inline void bytestream2_skipu(GetByteContext *g, >> static av_always_inline void bytestream2_skip_p(PutByteContext *p, >> unsigned int size) >> { >> - int size2; >> + unsigned int size2; >> if (p->eof) >> return; >> size2 = FFMIN(p->buffer_end - p->buffer, size); >> @@ -268,7 +268,7 @@ static av_always_inline unsigned int bytestream2_get_buffer(GetByteContext *g, >> uint8_t *dst, >> unsigned int size) >> { >> - int size2 = FFMIN(g->buffer_end - g->buffer, size); >> + unsigned int size2 = FFMIN(g->buffer_end - g->buffer, size); >> memcpy(dst, g->buffer, size2); >> g->buffer += size2; >> return size2; >> @@ -287,7 +287,7 @@ static av_always_inline unsigned int bytestream2_put_buffer(PutByteContext *p, >> const uint8_t *src, >> unsigned int size) >> { >> - int size2; >> + unsigned int size2; >> if (p->eof) >> return 0; >> size2 = FFMIN(p->buffer_end - p->buffer, size); >> @@ -311,7 +311,7 @@ static av_always_inline void bytestream2_set_buffer(PutByteContext *p, >> const uint8_t c, >> unsigned int size) >> { >> - int size2; >> + unsigned int size2; >> if (p->eof) >> return; >> size2 = FFMIN(p->buffer_end - p->buffer, size); >> @@ -348,7 +348,7 @@ static av_always_inline unsigned int bytestream2_copy_buffer(PutByteContext *p, >> GetByteContext *g, >> unsigned int size) >> { >> - int size2; >> + unsigned int size2; >> >> if (p->eof) >> return 0; > > The bytestream APIs are allowed to overread if the buffer is padded and > the user manages this himself. So you are not allowed to presume that > g->buffer_end - g->buffer is positive. I am not sure if overread/overwrote is a supported state for these functions. As far as I see bytestream2_get_buffer, bytestream2_put_buffer, bytestream2_copy_buffer and bytestream2_set_buffer just crashes if buffer_end < buffer because sooner or later memcpy/memset gets a negative value. There are no special checks to handle it. Regards, Marton _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h 2023-03-25 17:09 ` Marton Balint @ 2023-03-27 13:12 ` Devin Heitmueller 2023-03-27 13:26 ` Andreas Rheinhardt 1 sibling, 0 replies; 9+ messages in thread From: Devin Heitmueller @ 2023-03-27 13:12 UTC (permalink / raw) To: FFmpeg development discussions and patches On Sat, Mar 25, 2023 at 1:10 PM Marton Balint <cus@passwd.hu> wrote: > I am not sure if overread/overwrote is a supported state for these > functions. As far as I see bytestream2_get_buffer, bytestream2_put_buffer, > bytestream2_copy_buffer and bytestream2_set_buffer just crashes if > buffer_end < buffer because sooner or later memcpy/memset gets a negative > value. There are no special checks to handle it. This was the conclusion I came to as well. I couldn't imagine a case where it would ever actually work, since prior to my patch in every case it results in a call to memcpy() with a negative length argument. 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h 2023-03-25 17:09 ` Marton Balint 2023-03-27 13:12 ` Devin Heitmueller @ 2023-03-27 13:26 ` Andreas Rheinhardt 2023-03-28 19:05 ` Marton Balint 1 sibling, 1 reply; 9+ messages in thread From: Andreas Rheinhardt @ 2023-03-27 13:26 UTC (permalink / raw) To: ffmpeg-devel Marton Balint: > > > On Sat, 25 Mar 2023, Andreas Rheinhardt wrote: > >> Devin Heitmueller: >>> When including the header in decklink_enc.cpp it would be fed >>> through the C++ compiler rather than the C compiler, which has >>> more strict warnings when comparing signed/unsigned values. >>> >>> Make the local variables unsigned to match the arguments they are >>> being passed for those functions. >>> >>> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> >>> --- >>> libavcodec/bytestream.h | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h >>> index d0033f14f3..67080604b9 100644 >>> --- a/libavcodec/bytestream.h >>> +++ b/libavcodec/bytestream.h >>> @@ -180,7 +180,7 @@ static av_always_inline void >>> bytestream2_skipu(GetByteContext *g, >>> static av_always_inline void bytestream2_skip_p(PutByteContext *p, >>> unsigned int size) >>> { >>> - int size2; >>> + unsigned int size2; >>> if (p->eof) >>> return; >>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>> @@ -268,7 +268,7 @@ static av_always_inline unsigned int >>> bytestream2_get_buffer(GetByteContext *g, >>> uint8_t >>> *dst, >>> unsigned >>> int size) >>> { >>> - int size2 = FFMIN(g->buffer_end - g->buffer, size); >>> + unsigned int size2 = FFMIN(g->buffer_end - g->buffer, size); >>> memcpy(dst, g->buffer, size2); >>> g->buffer += size2; >>> return size2; >>> @@ -287,7 +287,7 @@ static av_always_inline unsigned int >>> bytestream2_put_buffer(PutByteContext *p, >>> const >>> uint8_t *src, >>> unsigned >>> int size) >>> { >>> - int size2; >>> + unsigned int size2; >>> if (p->eof) >>> return 0; >>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>> @@ -311,7 +311,7 @@ static av_always_inline void >>> bytestream2_set_buffer(PutByteContext *p, >>> const uint8_t c, >>> unsigned int size) >>> { >>> - int size2; >>> + unsigned int size2; >>> if (p->eof) >>> return; >>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>> @@ -348,7 +348,7 @@ static av_always_inline unsigned int >>> bytestream2_copy_buffer(PutByteContext *p, >>> >>> GetByteContext *g, >>> >>> unsigned int size) >>> { >>> - int size2; >>> + unsigned int size2; >>> >>> if (p->eof) >>> return 0; >> >> The bytestream APIs are allowed to overread if the buffer is padded and >> the user manages this himself. So you are not allowed to presume that >> g->buffer_end - g->buffer is positive. > > I am not sure if overread/overwrote is a supported state for these > functions. As far as I see bytestream2_get_buffer, > bytestream2_put_buffer, bytestream2_copy_buffer and > bytestream2_set_buffer just crashes if buffer_end < buffer because > sooner or later memcpy/memset gets a negative value. There are no > special checks to handle it. > True. Seems like this was never a supported case. Objection lifted. - Andreas _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h 2023-03-27 13:26 ` Andreas Rheinhardt @ 2023-03-28 19:05 ` Marton Balint 0 siblings, 0 replies; 9+ messages in thread From: Marton Balint @ 2023-03-28 19:05 UTC (permalink / raw) To: FFmpeg development discussions and patches On Mon, 27 Mar 2023, Andreas Rheinhardt wrote: > Marton Balint: >> >> >> On Sat, 25 Mar 2023, Andreas Rheinhardt wrote: >> >>> Devin Heitmueller: >>>> When including the header in decklink_enc.cpp it would be fed >>>> through the C++ compiler rather than the C compiler, which has >>>> more strict warnings when comparing signed/unsigned values. >>>> >>>> Make the local variables unsigned to match the arguments they are >>>> being passed for those functions. >>>> >>>> Signed-off-by: Devin Heitmueller <dheitmueller@ltnglobal.com> >>>> --- >>>> libavcodec/bytestream.h | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/libavcodec/bytestream.h b/libavcodec/bytestream.h >>>> index d0033f14f3..67080604b9 100644 >>>> --- a/libavcodec/bytestream.h >>>> +++ b/libavcodec/bytestream.h >>>> @@ -180,7 +180,7 @@ static av_always_inline void >>>> bytestream2_skipu(GetByteContext *g, >>>> static av_always_inline void bytestream2_skip_p(PutByteContext *p, >>>> unsigned int size) >>>> { >>>> - int size2; >>>> + unsigned int size2; >>>> if (p->eof) >>>> return; >>>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>>> @@ -268,7 +268,7 @@ static av_always_inline unsigned int >>>> bytestream2_get_buffer(GetByteContext *g, >>>> uint8_t >>>> *dst, >>>> unsigned >>>> int size) >>>> { >>>> - int size2 = FFMIN(g->buffer_end - g->buffer, size); >>>> + unsigned int size2 = FFMIN(g->buffer_end - g->buffer, size); >>>> memcpy(dst, g->buffer, size2); >>>> g->buffer += size2; >>>> return size2; >>>> @@ -287,7 +287,7 @@ static av_always_inline unsigned int >>>> bytestream2_put_buffer(PutByteContext *p, >>>> const >>>> uint8_t *src, >>>> unsigned >>>> int size) >>>> { >>>> - int size2; >>>> + unsigned int size2; >>>> if (p->eof) >>>> return 0; >>>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>>> @@ -311,7 +311,7 @@ static av_always_inline void >>>> bytestream2_set_buffer(PutByteContext *p, >>>> const uint8_t c, >>>> unsigned int size) >>>> { >>>> - int size2; >>>> + unsigned int size2; >>>> if (p->eof) >>>> return; >>>> size2 = FFMIN(p->buffer_end - p->buffer, size); >>>> @@ -348,7 +348,7 @@ static av_always_inline unsigned int >>>> bytestream2_copy_buffer(PutByteContext *p, >>>> >>>> GetByteContext *g, >>>> >>>> unsigned int size) >>>> { >>>> - int size2; >>>> + unsigned int size2; >>>> >>>> if (p->eof) >>>> return 0; >>> >>> The bytestream APIs are allowed to overread if the buffer is padded and >>> the user manages this himself. So you are not allowed to presume that >>> g->buffer_end - g->buffer is positive. >> >> I am not sure if overread/overwrote is a supported state for these >> functions. As far as I see bytestream2_get_buffer, >> bytestream2_put_buffer, bytestream2_copy_buffer and >> bytestream2_set_buffer just crashes if buffer_end < buffer because >> sooner or later memcpy/memset gets a negative value. There are no >> special checks to handle it. >> > > True. Seems like this was never a supported case. Objection lifted. Ok, will apply. Regards, Marton _______________________________________________ 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". ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-28 19:06 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-17 15:02 [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Devin Heitmueller 2023-03-17 15:02 ` [FFmpeg-devel] [PATCH v2 2/2] decklink: Add support for compressed AC-3 output over SDI Devin Heitmueller 2023-03-24 21:07 ` Marton Balint 2023-03-27 16:08 ` Devin Heitmueller 2023-03-25 4:47 ` [FFmpeg-devel] [PATCH v2 1/2] avcodec: Fix warnings with signed/unsigned compare in bitstream.h Andreas Rheinhardt 2023-03-25 17:09 ` Marton Balint 2023-03-27 13:12 ` Devin Heitmueller 2023-03-27 13:26 ` Andreas Rheinhardt 2023-03-28 19:05 ` Marton Balint
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