[1/2,v2] matroskaenc: fix muxing AAC streams when using aac_adtstoasc bsf

Message ID 20161102200231.568-1-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Nov. 2, 2016, 8:02 p.m.
aac_adtstoasc makes the aac extradata available only after the first packet
is filtered, and as packet side data.

Assume extradata will be available as part of the first packet if
avpriv_mpeg4audio_get_config() fails the first time due to missing extradata
and reserve space for the OutputSampleRate element in the Tracks master.

Signed-off-by: James Almer <jamrial@gmail.com>
---
No functionality change compared to the previous versio, just some refactoring
to better accommodate the following patch.

 libavformat/matroskaenc.c | 78 +++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 72 insertions(+), 6 deletions(-)

Comments

Anton Khirnov Nov. 9, 2016, 10:36 a.m. | #1
Quoting James Almer (2016-11-02 21:02:30)
> aac_adtstoasc makes the aac extradata available only after the first packet
> is filtered, and as packet side data.
> 
> Assume extradata will be available as part of the first packet if
> avpriv_mpeg4audio_get_config() fails the first time due to missing extradata
> and reserve space for the OutputSampleRate element in the Tracks master.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> No functionality change compared to the previous versio, just some refactoring
> to better accommodate the following patch.
> 
>  libavformat/matroskaenc.c | 78 +++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 72 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index cfced72..68c91fa 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -98,6 +98,8 @@ typedef struct MatroskaMuxContext {
>      int64_t         cluster_pts;
>      int64_t         duration_offset;
>      int64_t         duration;
> +    int64_t         sample_rate_offset;
> +    int             sample_rate;

This assumes there is at most one AAC track in the file, which is not
necessarily true. From a quick glance it seems to be rather
straigtforward to just move this to the per-track context.
James Almer Nov. 9, 2016, 3:36 p.m. | #2
On 11/9/2016 7:36 AM, Anton Khirnov wrote:
> Quoting James Almer (2016-11-02 21:02:30)
>> aac_adtstoasc makes the aac extradata available only after the first packet
>> is filtered, and as packet side data.
>>
>> Assume extradata will be available as part of the first packet if
>> avpriv_mpeg4audio_get_config() fails the first time due to missing extradata
>> and reserve space for the OutputSampleRate element in the Tracks master.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> No functionality change compared to the previous versio, just some refactoring
>> to better accommodate the following patch.
>>
>>  libavformat/matroskaenc.c | 78 +++++++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 72 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index cfced72..68c91fa 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -98,6 +98,8 @@ typedef struct MatroskaMuxContext {
>>      int64_t         cluster_pts;
>>      int64_t         duration_offset;
>>      int64_t         duration;
>> +    int64_t         sample_rate_offset;
>> +    int             sample_rate;
> 
> This assumes there is at most one AAC track in the file, which is not
> necessarily true. From a quick glance it seems to be rather
> straigtforward to just move this to the per-track context.

Mmh, that's true. Same with the second patch.
Thanks for pointing it out. Will see about making it work in per track
basis.

Patch

diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index cfced72..68c91fa 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -98,6 +98,8 @@  typedef struct MatroskaMuxContext {
     int64_t         cluster_pts;
     int64_t         duration_offset;
     int64_t         duration;
+    int64_t         sample_rate_offset;
+    int             sample_rate;
     mkv_seekhead    *main_seekhead;
     mkv_cues        *cues;
     mkv_track       *tracks;
@@ -521,20 +523,36 @@  static int put_flac_codecpriv(AVFormatContext *s,
     return 0;
 }
 
-static int get_aac_sample_rates(AVFormatContext *s, AVCodecParameters *par,
+static int get_aac_sample_rates(AVFormatContext *s, uint8_t *extradata, int extradata_size,
                                 int *sample_rate, int *output_sample_rate)
 {
     MPEG4AudioConfig mp4ac;
+    int ret;
 
-    if (avpriv_mpeg4audio_get_config(&mp4ac, par->extradata,
-                                     par->extradata_size * 8, 1) < 0) {
+    ret = avpriv_mpeg4audio_get_config(&mp4ac, extradata,
+                                       extradata_size * 8, 1);
+    /* Don't abort if the failure is because of missing extradata. Assume in that
+     * case a bitstream filter will provide the muxer with the extradata in the
+     * first packet.
+     * Abort however if s->pb is not seekable, as we would not be able to seek back
+     * to write the sample rate elements once the extradata shows up, anyway. */
+    if (ret < 0 && (extradata_size || !(s->pb->seekable & AVIO_SEEKABLE_NORMAL))) {
         av_log(s, AV_LOG_ERROR,
                "Error parsing AAC extradata, unable to determine samplerate.\n");
         return AVERROR(EINVAL);
     }
 
-    *sample_rate        = mp4ac.sample_rate;
-    *output_sample_rate = mp4ac.ext_sample_rate;
+    if (ret < 0) {
+        /* This will only happen when this function is called while writing the
+         * header and no extradata is available. The space for this element has
+         * to be reserved for when this function is called again after the
+         * extradata shows up in the first packet, as there's no way to know if
+         * output_sample_rate will be different than sample_rate or not. */
+        *output_sample_rate = *sample_rate;
+    } else {
+        *sample_rate        = mp4ac.sample_rate;
+        *output_sample_rate = mp4ac.ext_sample_rate;
+    }
     return 0;
 }
 
@@ -790,7 +808,8 @@  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
         bit_depth = av_get_bytes_per_sample(par->format) << 3;
 
     if (par->codec_id == AV_CODEC_ID_AAC) {
-        ret = get_aac_sample_rates(s, par, &sample_rate, &output_sample_rate);
+        ret = get_aac_sample_rates(s, par->extradata, par->extradata_size, &sample_rate,
+                                   &output_sample_rate);
         if (ret < 0)
             return ret;
     }
@@ -890,6 +909,8 @@  static int mkv_write_track(AVFormatContext *s, MatroskaMuxContext *mkv,
 
         subinfo = start_ebml_master(pb, MATROSKA_ID_TRACKAUDIO, 0);
         put_ebml_uint  (pb, MATROSKA_ID_AUDIOCHANNELS    , par->channels);
+
+        mkv->sample_rate_offset = avio_tell(pb);
         put_ebml_float (pb, MATROSKA_ID_AUDIOSAMPLINGFREQ, sample_rate);
         if (output_sample_rate)
             put_ebml_float(pb, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
@@ -1531,6 +1552,47 @@  static void mkv_flush_dynbuf(AVFormatContext *s)
     mkv->dyn_bc = NULL;
 }
 
+static int mkv_check_new_extra_data(AVFormatContext *s, AVPacket *pkt)
+{
+    MatroskaMuxContext *mkv = s->priv_data;
+    AVCodecParameters *par  = s->streams[pkt->stream_index]->codecpar;
+    uint8_t *side_data;
+    int side_data_size = 0, ret;
+
+    side_data = av_packet_get_side_data(pkt, AV_PKT_DATA_NEW_EXTRADATA,
+                                                     &side_data_size);
+
+    switch (par->codec_id) {
+    case AV_CODEC_ID_AAC:
+        if (side_data_size && (s->pb->seekable & AVIO_SEEKABLE_NORMAL)) {
+            int output_sample_rate = 0;
+            int64_t curpos;
+            ret = get_aac_sample_rates(s, side_data, side_data_size, &mkv->sample_rate,
+                                       &output_sample_rate);
+            if (ret < 0)
+                return ret;
+            if (!output_sample_rate)
+                output_sample_rate = mkv->sample_rate; // Space is already reserved, so it's this or a void element.
+            curpos = avio_tell(s->pb);
+            avio_seek(s->pb, mkv->sample_rate_offset, SEEK_SET);
+            put_ebml_float(s->pb, MATROSKA_ID_AUDIOSAMPLINGFREQ, mkv->sample_rate);
+            put_ebml_float(s->pb, MATROSKA_ID_AUDIOOUTSAMPLINGFREQ, output_sample_rate);
+            avio_seek(s->pb, curpos, SEEK_SET);
+        } else if (!par->extradata_size && !mkv->sample_rate) {
+            // No extradata (codecpar or packet side data).
+            av_log(s, AV_LOG_ERROR, "Error parsing AAC extradata, unable to determine samplerate.\n");
+            return AVERROR(EINVAL);
+        }
+        break;
+    default:
+        if (side_data_size)
+            av_log(s, AV_LOG_DEBUG, "Ignoring new extradata in a packet for stream %d.\n", pkt->stream_index);
+        break;
+    }
+
+    return 0;
+}
+
 static int mkv_write_packet_internal(AVFormatContext *s, AVPacket *pkt)
 {
     MatroskaMuxContext *mkv = s->priv_data;
@@ -1605,6 +1667,10 @@  static int mkv_write_packet(AVFormatContext *s, AVPacket *pkt)
     AVIOContext *pb;
     int ret;
 
+    ret = mkv_check_new_extra_data(s, pkt);
+    if (ret < 0)
+        return ret;
+
     if (mkv->tracks[pkt->stream_index].write_dts)
         cluster_time = pkt->dts - mkv->cluster_pts;
     else