diff mbox series

[FFmpeg-devel,1/4] avformat/audiointerleave: disallow using a samples_per_frame array

Message ID 20200228003750.22536-1-cus@passwd.hu
State Superseded
Headers show
Series [FFmpeg-devel,1/4] avformat/audiointerleave: disallow using a samples_per_frame array | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Marton Balint Feb. 28, 2020, 12:37 a.m. UTC 
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@passwd.hu>
---
 libavformat/audiointerleave.c | 24 ++++++++++--------------
 libavformat/audiointerleave.h |  7 ++++---
 libavformat/gxfenc.c          |  2 +-
 libavformat/mxfenc.c          |  7 ++-----
 4 files changed, 17 insertions(+), 23 deletions(-)

Comments

Tomas Härdin March 2, 2020, 5 p.m. UTC | #1 
Sorry for replying a bit late, I've been sick

fre 2020-02-28 klockan 01:37 +0100 skrev Marton Balint:
> Only MXF used an actual sample array, and that is unneeded there
> because simple
> rounding rules can be used instead.

Does this produce the exact same rounding? Like 1602, 1601,
 1602, 1601, 1602, 1062 ... for 30/1.001 fps? If so then this is a nice
solution


> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/audiointerleave.c | 24 ++++++++++--------------
>  libavformat/audiointerleave.h |  7 ++++---
>  libavformat/gxfenc.c          |  2 +-
>  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);

Looks correct

>              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;
>  
> -            aic->fifo_size = 100* *aic->samples;
> -            if (!(aic->fifo= av_fifo_alloc_array(100, *aic->samples)))
> +            aic->fifo_size = 100 * max_samples;
> +            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);

Here's where the meat is. av_rescale_q() means AV_ROUND_NEAR_INF. So
basically round(). Fiddling around a bit in Octave to replicate the
logic:

  octave:13> round(48000*1001/30000)
  ans =  1602
  octave:14> round(48000*1001/30000*2)-1602
  ans =  1601
  octave:15> round(48000*1001/30000*3)-(1602+1601)
  ans =  1602
  octave:16> round(48000*1001/30000*4)-(1602+1601+1602)
  ans =  1601
  octave:17> 48000*1001/30000*5-(1602+1601+1602+1601)
  ans =  1602

Seems kosher. The rest of the patch looks OK.

mxfdec.c could also make use of this. Then we could remove
ff_mxf_get_samples_per_frame() and related code. Replacing
mxf_content_package_rates with a simple formula would also be nice.

/Tomas
diff mbox series

Patch

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);
             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;
 
-            aic->fifo_size = 100* *aic->samples;
-            if (!(aic->fifo= av_fifo_alloc_array(100, *aic->samples)))
+            aic->fifo_size = 100 * max_samples;
+            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;