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

2018-09-05 Thread Markus Schumann


> On Sep 5, 2018, at 16:06, Marton Balint  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@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.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


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

2018-09-04 Thread Markus Schumann
---
 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;
 }

--
2.17.1


___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel