Message ID | 20230823095505.490814-1-ulrik.mikaelsson@gmail.com |
---|---|
State | New |
Headers | show |
Series | [FFmpeg-devel,v2] avformat/mp3dec: Subtract known padding from duration | expand |
Context | Check | Description |
---|---|---|
yinshiyou/make_loongarch64 | success | Make finished |
yinshiyou/make_fate_loongarch64 | fail | Make fate failed |
andriy/make_x86 | success | Make finished |
andriy/make_fate_x86 | fail | Make fate failed |
ons 2023-08-23 klockan 11:55 +0200 skrev Ulrik Mikaelsson: > When an Info-tag is present, marking initial and trailing samples as > padding, those samples should not be included in the calculation of > track > duration. > > This solves a surprising user experience where converting a WAV->MP3- > >WAV, > ffprobe will show the duration of the mp3 as slightly longer than > both the > input and the output. > --- > libavformat/mp3dec.c | 3 ++- > tests/ref/fate/gapless-mp3-side-data | 4 ++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c > index 05c13228bc..54947f521e 100644 > --- a/libavformat/mp3dec.c > +++ b/libavformat/mp3dec.c > @@ -350,7 +350,8 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, > AVStream *st, int64_t base) > avio_seek(s->pb, base + vbrtag_size, SEEK_SET); > > if (mp3->frames) > - st->duration = av_rescale_q(mp3->frames, (AVRational){spf, > c.sample_rate}, > + st->duration = av_rescale_q(((uint64_t)mp3->frames * > (uint64_t)spf) - mp3->start_pad - mp3->end_pad, > + (AVRational){1, c.sample_rate}, > st->time_base); > if (mp3->header_filesize && mp3->frames && !mp3->is_cbr) > st->codecpar->bit_rate = av_rescale(mp3->header_filesize, 8 > * c.sample_rate, mp3->frames * (int64_t)spf); > diff --git a/tests/ref/fate/gapless-mp3-side-data > b/tests/ref/fate/gapless-mp3-side-data > index caf42068dc..495a5bb865 100644 > --- a/tests/ref/fate/gapless-mp3-side-data > +++ b/tests/ref/fate/gapless-mp3-side-data > @@ -596,5 +596,5 @@ > packet|codec_type=audio|stream_index=0|pts=218603520|pts_time=15.4906 > 12|dts=2186 > > packet|codec_type=audio|stream_index=0|pts=218972160|pts_time=15.516 > 735|dts=218972160|dts_time=15.516735|duration=368640|duration_time=0. > 026122|size=418|pos=249718|flags=K__|data_hash=CRC32:3789f3cf|side_da > ta|side_data_type=Skip > Samples|skip_samples=0|discard_padding=1152|skip_reason=0|discard_rea > son=0 > > - > stream|index=0|codec_name=mp3|profile=unknown|codec_type=audio|codec_t > ag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=4 > 4100|channels=2|channel_layout=stereo|bits_per_sample=0|initial_paddi > ng=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/14112000| > start_pts=353600|start_time=0.025057|duration_ts=219340800|duration=1 > 5.542857|bit_rate=128000|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_ > frames=N/A|nb_read_frames=N/A|nb_read_packets=595|disposition:default > =0|disposition:dub=0|disposition:original=0|disposition:comment=0|dis > position:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposit > ion:hearing_impaired=0|disposition:visual_impaired=0|disposition:clea > n_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0 > |disposition:captions=0|disposition:descriptions=0|disposition:metada > ta=0|disposition:dependent=0|disposition:still_image=0|tag:encoder=LA > ME3.93 > - > format|filename=gapless.mp3|nb_streams=1|nb_programs=0|format_name=mp3 > |start_time=0.025057|duration=15.542857|size=250264|bit_rate=128812|p > robe_score=51|tag:title=test > +stream|index=0|codec_name=mp3|profile=unknown|codec_type=audio|codec > _tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate > =44100|channels=2|channel_layout=stereo|bits_per_sample=0|initial_pad > ding=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/1411200 > 0|start_pts=353600|start_time=0.025057|duration_ts=218521600|duration > =15.484807|bit_rate=128000|max_bit_rate=N/A|bits_per_raw_sample=N/A|n > b_frames=N/A|nb_read_frames=N/A|nb_read_packets=595|disposition:defau > lt=0|disposition:dub=0|disposition:original=0|disposition:comment=0|d > isposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|dispos > ition:hearing_impaired=0|disposition:visual_impaired=0|disposition:cl > ean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails > =0|disposition:captions=0|disposition:descriptions=0|disposition:meta > data=0|disposition:dependent=0|disposition:still_image=0|tag:encoder= > LAME3.93 > +format|filename=gapless.mp3|nb_streams=1|nb_programs=0|format_name=m > p3|start_time=0.025057|duration=15.484807|size=250264|bit_rate=129295 > |probe_score=51|tag:title=test This still fails fate-seek-extra-mp3 /Tomas
> This still fails fate-seek-extra-mp3
I noticed. Apologies for the noise, I'm unfamiliar with the FATE suite
and failed to realize "make fate" won't run the _whole_ thing.
I have just submitted a 3d revision of this patch, fixing the review
comments and fully passing `make fate SAMPLES=./fate-sui
te`.
diff --git a/libavformat/mp3dec.c b/libavformat/mp3dec.c index 05c13228bc..54947f521e 100644 --- a/libavformat/mp3dec.c +++ b/libavformat/mp3dec.c @@ -350,7 +350,8 @@ static int mp3_parse_vbr_tags(AVFormatContext *s, AVStream *st, int64_t base) avio_seek(s->pb, base + vbrtag_size, SEEK_SET); if (mp3->frames) - st->duration = av_rescale_q(mp3->frames, (AVRational){spf, c.sample_rate}, + st->duration = av_rescale_q(((uint64_t)mp3->frames * (uint64_t)spf) - mp3->start_pad - mp3->end_pad, + (AVRational){1, c.sample_rate}, st->time_base); if (mp3->header_filesize && mp3->frames && !mp3->is_cbr) st->codecpar->bit_rate = av_rescale(mp3->header_filesize, 8 * c.sample_rate, mp3->frames * (int64_t)spf); diff --git a/tests/ref/fate/gapless-mp3-side-data b/tests/ref/fate/gapless-mp3-side-data index caf42068dc..495a5bb865 100644 --- a/tests/ref/fate/gapless-mp3-side-data +++ b/tests/ref/fate/gapless-mp3-side-data @@ -596,5 +596,5 @@ packet|codec_type=audio|stream_index=0|pts=218603520|pts_time=15.490612|dts=2186 packet|codec_type=audio|stream_index=0|pts=218972160|pts_time=15.516735|dts=218972160|dts_time=15.516735|duration=368640|duration_time=0.026122|size=418|pos=249718|flags=K__|data_hash=CRC32:3789f3cf|side_data|side_data_type=Skip Samples|skip_samples=0|discard_padding=1152|skip_reason=0|discard_reason=0 -stream|index=0|codec_name=mp3|profile=unknown|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=44100|channels=2|channel_layout=stereo|bits_per_sample=0|initial_padding=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/14112000|start_pts=353600|start_time=0.025057|duration_ts=219340800|duration=15.542857|bit_rate=128000|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=595|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0|tag:encoder=LAME3.93 -format|filename=gapless.mp3|nb_streams=1|nb_programs=0|format_name=mp3|start_time=0.025057|duration=15.542857|size=250264|bit_rate=128812|probe_score=51|tag:title=test +stream|index=0|codec_name=mp3|profile=unknown|codec_type=audio|codec_tag_string=[0][0][0][0]|codec_tag=0x0000|sample_fmt=fltp|sample_rate=44100|channels=2|channel_layout=stereo|bits_per_sample=0|initial_padding=0|id=N/A|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/14112000|start_pts=353600|start_time=0.025057|duration_ts=218521600|duration=15.484807|bit_rate=128000|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=595|disposition:default=0|disposition:dub=0|disposition:original=0|disposition:comment=0|disposition:lyrics=0|disposition:karaoke=0|disposition:forced=0|disposition:hearing_impaired=0|disposition:visual_impaired=0|disposition:clean_effects=0|disposition:attached_pic=0|disposition:timed_thumbnails=0|disposition:captions=0|disposition:descriptions=0|disposition:metadata=0|disposition:dependent=0|disposition:still_image=0|tag:encoder=LAME3.93 +format|filename=gapless.mp3|nb_streams=1|nb_programs=0|format_name=mp3|start_time=0.025057|duration=15.484807|size=250264|bit_rate=129295|probe_score=51|tag:title=test