[FFmpeg-devel] [PATCH] fix MXF audio PTS calculation for compressed audio (ADTS/AAC)

[Markus Schumann]

> On Sep 5, 2018, at 16:06, Marton Balint <cus at passwd.hu> wrote:
> 
> 
> 
>> On Tue, 4 Sep 2018, Markus Schumann wrote:
>> 
>> ---
>> libavformat/mxfdec.c | 67 +++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 66 insertions(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/mxfdec.c b/libavformat/mxfdec.c
>> index 8e1089620f..adfffd954a
>> --- a/libavformat/mxfdec.c
>> +++ b/libavformat/mxfdec.c
>> @@ -3266,9 +3266,63 @@ static int64_t mxf_set_current_edit_unit(MXFContext *mxf, AVStream *st, int64_t
>>    return mxf_set_current_edit_unit(mxf, st, current_offset, 0);
>> }
>> 
>> +static int mxf_aac_adts_count_frames(MXFContext *mxf, AVCodecParameters *par,
>> +                                     AVPacket *pkt)
>> +{
>> +    const uint8_t *end_ptr;
>> +    uint8_t *data_ptr;
>> +    int32_t aac_frame_length, adts_frame_count;
>> +    uint8_t sample_rate_index;
>> +    uint8_t channel_configuration;
>> +
>> +    data_ptr = pkt->data;
>> +    end_ptr = pkt->data + pkt->size;
>> +       adts_frame_count = 0;
>> +
>> +       /* ADTS header is 7 bytes */
>> +       if (pkt->size < 7)
>> +               return 0;
>> +
>> +       /* check for sync bits 0xfff */
>> +       if (data_ptr[0] != 0xff || (data_ptr[1] & 0xf0) != 0xf0) return AVERROR(EINVAL);
>> +
>> +       sample_rate_index = (data_ptr[2] & 0x3c) >> 2;
>> +
>> +       channel_configuration = ((data_ptr[2] & 0x01) << 2) | ((data_ptr[3] & 0xc0) >> 6);
>> +
>> +       adts_frame_count = 1;
>> +
>> +       data_ptr += 7;
>> +
>> +       for (; data_ptr + 7 < end_ptr; ++data_ptr)
>> +       {
>> +               /* check for sync bits 0xfff */
>> +               if (data_ptr[0] != 0xff || (data_ptr[1] & 0xf0) != 0xf0) continue;
>> +
>> +               /* make sure sample rate is identical */
>> +               if (sample_rate_index != (data_ptr[2] & 0x3c) >> 2) continue;
>> +
>> +               /* make sure channel configuration is identical */
>> +               if (channel_configuration != (((data_ptr[2] & 0x01) << 2) | ((data_ptr[3] & 0xc0) >> 6))) continue;
>> +
>> +               aac_frame_length = ((int32_t)(data_ptr[3] & 0x03) << 11) | ((int32_t)data_ptr[4] << 3) | ((int32_t)(data_ptr[5] & 0xe0) >> 5);
>> +
>> +               /* sanity check on the frame length */
>> +               if (aac_frame_length < 1 || aac_frame_length > 8 * 768 + 7 + 2) continue;
>> +
>> +               ++adts_frame_count;
>> +
>> +               data_ptr += 7;
>> +       }
>> +
>> +       return adts_frame_count;
>> +}
>> +
>> static int mxf_set_audio_pts(MXFContext *mxf, AVCodecParameters *par,
>>                             AVPacket *pkt)
>> {
>> +    int frame_count;
>> +
>>    MXFTrack *track = mxf->fc->streams[pkt->stream_index]->priv_data;
>>    int64_t bits_per_sample = par->bits_per_coded_sample;
>> 
>> @@ -3281,7 +3335,18 @@ static int mxf_set_audio_pts(MXFContext *mxf, AVCodecParameters *par,
>>        || bits_per_sample <= 0
>>        || par->channels * (int64_t)bits_per_sample < 8)
>>        return AVERROR(EINVAL);
>> -    track->sample_count += pkt->size / (par->channels * (int64_t)bits_per_sample / 8);
>> +
>> +    switch (par->codec_id) {
>> +    case AV_CODEC_ID_AAC:
>> +        frame_count =  mxf_aac_adts_count_frames(mxf, par, pkt);
>> +        if (frame_count < 0) return AVERROR(EINVAL);
>> +        track->sample_count += 1024 * frame_count;
>> +       break;
>> +    default:
>> +       /* here we assume PCM */
>> +       track->sample_count += pkt->size / (par->channels * (int64_t)bits_per_sample / 8);
>> +       break;
>> +    }
>>    return 0;
> 
> I just pushed a patch which fixes the error messages decoding frame wrapped (as in 25fps frame wrapped) AAC packets. Obviously audio packet pts-es will not be sample correct, but they should be accurate enough for average use cases. (and it is not uncommon that demuxers are unable to provide sample accurate timestamps).
> 
> In general it is not a good idea to put codec parsing code in a demuxer, we have parsers for that. So an alternative approach might be to return AV_NOPTS_VALUE from the mxf demuxer and let the aac parser fill in the timestamps, but in this case, since we can determine a rough timestamp based on the format, I think it is better if we return that, because that is what the other demuxers are doing as well.
> 
> Is there a use case which does not work for you using the current git master?
> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

I just built FFmpeg master with your changes and then VLC using FFmpeg master. 

My sample from ticket #7366 plays fine in VLC with your changes.

So your changes should be good enough.

Thanks Markus.