[FFmpeg-devel] [PATCH 1/4] avformat/audiointerleave: disallow using a samples_per_frame array
Andreas Rheinhardt
andreas.rheinhardt at gmail.com
Mon Mar 2 19:16:42 EET 2020
On Fri, Feb 28, 2020 at 1:38 AM Marton Balint <cus at passwd.hu> wrote:
> Only MXF used an actual sample array, and that is unneeded there because
> simple
> rounding rules can be used instead.
>
> Signed-off-by: Marton Balint <cus at passwd.hu>
> ---
> libavformat/audiointerleave.c | 24 ++++++++++--------------
> libavformat/audiointerleave.h | 7 ++++---
> libavformat/gxfenc.c | 2 +- libavformat/mxfenc.c | 7
> ++-----
>
> libavformat/mxfenc.c | 7 ++-----
> 4 files changed, 17 insertions(+), 23 deletions(-)
>
> diff --git a/libavformat/audiointerleave.c b/libavformat/audiointerleave.c
> index 6797546a44..f9887c1f4a 100644
> --- a/libavformat/audiointerleave.c
> +++ b/libavformat/audiointerleave.c
> @@ -39,14 +39,11 @@ void ff_audio_interleave_close(AVFormatContext *s)
> }
>
> int ff_audio_interleave_init(AVFormatContext *s,
> - const int *samples_per_frame,
> + const int samples_per_frame,
> AVRational time_base)
> {
> int i;
>
> - if (!samples_per_frame)
> - return AVERROR(EINVAL);
> -
> if (!time_base.num) {
> av_log(s, AV_LOG_ERROR, "timebase not set for audio
> interleave\n");
> return AVERROR(EINVAL);
> @@ -56,18 +53,18 @@ int ff_audio_interleave_init(AVFormatContext *s,
> AudioInterleaveContext *aic = st->priv_data;
>
> if (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) {
> + int max_samples = samples_per_frame ? samples_per_frame :
> av_rescale_rnd(st->codecpar->sample_rate, time_base.num, time_base.den,
> AV_ROUND_UP);
>
This line and a few others are overly long. Why don't you split them?
> aic->sample_size = (st->codecpar->channels *
>
> av_get_bits_per_sample(st->codecpar->codec_id)) / 8;
> if (!aic->sample_size) {
> av_log(s, AV_LOG_ERROR, "could not compute sample
> size\n");
> return AVERROR(EINVAL);
> }
> - aic->samples_per_frame = samples_per_frame;
> - aic->samples = aic->samples_per_frame;
> aic->time_base = time_base;
> + aic->samples_per_frame = samples_per_frame;
>
I don't see a reason to move this line and not moving it would lead to a
smaller diff.
>
> - aic->fifo_size = 100* *aic->samples;
> - if (!(aic->fifo= av_fifo_alloc_array(100, *aic->samples)))
> + aic->fifo_size = 100 * max_samples;
>
It doesn't matter here, but I think it is generally better to only set the
size after you know the allocation succeeded.
> + if (!(aic->fifo = av_fifo_alloc_array(100, max_samples)))
> return AVERROR(ENOMEM);
> }
> }
> @@ -81,7 +78,8 @@ static int interleave_new_audio_packet(AVFormatContext
> *s, AVPacket *pkt,
> AVStream *st = s->streams[stream_index];
> AudioInterleaveContext *aic = st->priv_data;
> int ret;
> - int frame_size = *aic->samples * aic->sample_size;
> + int nb_samples = aic->samples_per_frame ? aic->samples_per_frame :
> (av_rescale_q(aic->n + 1, av_make_q(st->codecpar->sample_rate, 1),
> av_inv_q(aic->time_base)) - aic->nb_samples);
> + int frame_size = nb_samples * aic->sample_size;
> int size = FFMIN(av_fifo_size(aic->fifo), frame_size);
> if (!size || (!flush && size == av_fifo_size(aic->fifo)))
> return 0;
> @@ -95,13 +93,11 @@ static int interleave_new_audio_packet(AVFormatContext
> *s, AVPacket *pkt,
> memset(pkt->data + size, 0, pkt->size - size);
>
> pkt->dts = pkt->pts = aic->dts;
> - pkt->duration = av_rescale_q(*aic->samples, st->time_base,
> aic->time_base);
> + pkt->duration = av_rescale_q(nb_samples, st->time_base,
> aic->time_base);
> pkt->stream_index = stream_index;
> aic->dts += pkt->duration;
> -
> - aic->samples++;
> - if (!*aic->samples)
> - aic->samples = aic->samples_per_frame;
> + aic->nb_samples += nb_samples;
> + aic->n++;
>
> return pkt->size;
> }
> diff --git a/libavformat/audiointerleave.h b/libavformat/audiointerleave.h
> index f28d5fefcc..0933310f4c 100644
> --- a/libavformat/audiointerleave.h
> +++ b/libavformat/audiointerleave.h
> @@ -29,14 +29,15 @@
> typedef struct AudioInterleaveContext {
> AVFifoBuffer *fifo;
> unsigned fifo_size; ///< size of currently allocated FIFO
> + int64_t n; ///< number of generated packets
> + int64_t nb_samples; ///< number of generated samples
> uint64_t dts; ///< current dts
> int sample_size; ///< size of one sample all channels
> included
> - const int *samples_per_frame; ///< must be 0-terminated
> - const int *samples; ///< current samples per frame, pointer
> to samples_per_frame
> + int samples_per_frame; ///< samples per frame if fixed, 0
> otherwise
> AVRational time_base; ///< time base of output audio packets
> } AudioInterleaveContext;
>
> -int ff_audio_interleave_init(AVFormatContext *s, const int
> *samples_per_frame, AVRational time_base);
> +int ff_audio_interleave_init(AVFormatContext *s, const int
> samples_per_frame, AVRational time_base);
> void ff_audio_interleave_close(AVFormatContext *s);
>
> /**
> diff --git a/libavformat/gxfenc.c b/libavformat/gxfenc.c
> index 9eebefc683..e7536a6a7e 100644
> --- a/libavformat/gxfenc.c
> +++ b/libavformat/gxfenc.c
> @@ -663,7 +663,7 @@ static int gxf_write_umf_packet(AVFormatContext *s)
> return updatePacketSize(pb, pos);
> }
>
> -static const int GXF_samples_per_frame[] = { 32768, 0 };
> +static const int GXF_samples_per_frame = 32768;
>
> static void gxf_init_timecode_track(GXFStreamContext *sc,
> GXFStreamContext *vsc)
> {
> diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> index 7ea47d7311..d77f993947 100644
> --- a/libavformat/mxfenc.c
> +++ b/libavformat/mxfenc.c
> @@ -1747,7 +1747,7 @@ static void
> mxf_write_index_table_segment(AVFormatContext *s)
> avio_wb32(pb, KAG_SIZE); // system item size including klv
> fill
> } else { // audio or data track
> if (!audio_frame_size) {
> - audio_frame_size = sc->aic.samples[0]*sc->aic.sample_size;
> + audio_frame_size = sc->frame_size;
> audio_frame_size += klv_fill_size(audio_frame_size);
> }
> avio_w8(pb, 1);
> @@ -2650,10 +2650,7 @@ static int mxf_write_header(AVFormatContext *s)
> return AVERROR(ENOMEM);
> mxf->timecode_track->index = -1;
>
> - if (!spf)
> - spf = ff_mxf_get_samples_per_frame(s, (AVRational){ 1, 25 });
> -
> - if (ff_audio_interleave_init(s, spf->samples_per_frame,
> mxf->time_base) < 0)
> + if (ff_audio_interleave_init(s, 0, s->oformat == &ff_mxf_opatom_muxer
> ? av_inv_q(mxf->audio_edit_rate) : mxf->time_base) < 0)
> return -1;
>
> return 0;
> --
> 2.16.4
More information about the ffmpeg-devel
mailing list