| Message ID | 20230620050932.85936-1-toots@rastageeks.org |
|---|---|
| State | Accepted |
| Commit | 468615f2045da325e0f73e8e668d49cf456ccb37 |
| Headers | show |
| Series | [FFmpeg-devel] libavformat/mpegts.c: fix hardcoded 5-bytes skip for metadata streams. | 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 |
On Tue, Jun 20, 2023 at 7:19 AM <toots@rastageeks.org> wrote: > From: Romain Beauxis <toots@rastageeks.org> > > Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata > streams > in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), > AV_CODEC_ID_SMPTE_KLV > was the only existing codec for metadata. > > It seems that this codec has a 5-bytes metadata header[1] that, for some > reason, > was always skipped when decoding data packets. > > However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results > in the > 5 first bytes of the payload being cut-off, which includes essential > informations > such as the ID3 tag version. > > This patch fixes the issue by keeping the 5-bytes skip only for > AV_CODEC_ID_SMPTE_KLV > streams. > > To test: > 1. download this file: > https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 > > This file was download from: > http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 > > 2. run this command: > ffprobe -show_streams -select_streams 0 -show_packets -show_private_data > \ > -show_data /path/to/bla.ts > > Before: > [PACKET] > codec_type=data > stream_index=0 > pts=494646418 > pts_time=5496.071311 > dts=494646418 > dts_time=5496.071311 > duration=N/A > duration_time=N/A > size=21 > pos=482784 > flags=K__ > data= > 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... > 00000010: 7465 7374 00 test. > > After: > [PACKET] > codec_type=data > stream_index=0 > pts=494646418 > pts_time=5496.071311 > dts=494646418 > dts_time=5496.071311 > duration=N/A > duration_time=N/A > size=26 > pos=482784 > flags=K__ > data= > 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. > 00000010: 0006 0000 0374 6573 7400 .....test. > > --- > libavformat/mpegts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c > index fb8b0bf8fd..0b3edda817 100644 > --- a/libavformat/mpegts.c > +++ b/libavformat/mpegts.c > @@ -1305,7 +1305,7 @@ skip: > p += sl_header_bytes; > buf_size -= sl_header_bytes; > } > - if (pes->stream_type == 0x15 && buf_size >= 5) { > + if (pes->st->codecpar->codec_id == AV_CODEC_ID_SMPTE_KLV > && buf_size >= 5) { > /* skip metadata access unit header */ > pes->pes_header_size += 5; > p += 5; > -- > 2.39.2 (Apple Git-143) > LGTM > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
Le mar. 20 juin 2023 à 02:10, Paul B Mahol <onemda@gmail.com> a écrit : > > > > On Tue, Jun 20, 2023 at 7:19 AM <toots@rastageeks.org> wrote: >> >> From: Romain Beauxis <toots@rastageeks.org> >> >> Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata streams >> in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), AV_CODEC_ID_SMPTE_KLV >> was the only existing codec for metadata. >> >> It seems that this codec has a 5-bytes metadata header[1] that, for some reason, >> was always skipped when decoding data packets. >> >> However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results in the >> 5 first bytes of the payload being cut-off, which includes essential informations >> such as the ID3 tag version. >> >> This patch fixes the issue by keeping the 5-bytes skip only for AV_CODEC_ID_SMPTE_KLV >> streams. >> >> To test: >> 1. download this file: https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 >> >> This file was download from: http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 >> >> 2. run this command: >> ffprobe -show_streams -select_streams 0 -show_packets -show_private_data \ >> -show_data /path/to/bla.ts >> >> Before: >> [PACKET] >> codec_type=data >> stream_index=0 >> pts=494646418 >> pts_time=5496.071311 >> dts=494646418 >> dts_time=5496.071311 >> duration=N/A >> duration_time=N/A >> size=21 >> pos=482784 >> flags=K__ >> data= >> 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... >> 00000010: 7465 7374 00 test. >> >> After: >> [PACKET] >> codec_type=data >> stream_index=0 >> pts=494646418 >> pts_time=5496.071311 >> dts=494646418 >> dts_time=5496.071311 >> duration=N/A >> duration_time=N/A >> size=26 >> pos=482784 >> flags=K__ >> data= >> 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. >> 00000010: 0006 0000 0374 6573 7400 .....test. >> >> --- >> libavformat/mpegts.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c >> index fb8b0bf8fd..0b3edda817 100644 >> --- a/libavformat/mpegts.c >> +++ b/libavformat/mpegts.c >> @@ -1305,7 +1305,7 @@ skip: >> p += sl_header_bytes; >> buf_size -= sl_header_bytes; >> } >> - if (pes->stream_type == 0x15 && buf_size >= 5) { >> + if (pes->st->codecpar->codec_id == AV_CODEC_ID_SMPTE_KLV && buf_size >= 5) { >> /* skip metadata access unit header */ >> pes->pes_header_size += 5; >> p += 5; >> -- >> 2.39.2 (Apple Git-143) > > > LGTM Great, thanks! Anything I can do to help move this to the finish line? Thanks, -- Romain >> >> >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 6/20/2023 8:58 PM, Romain Beauxis wrote: > Le mar. 20 juin 2023 à 02:10, Paul B Mahol <onemda@gmail.com> a écrit : >> >> >> >> On Tue, Jun 20, 2023 at 7:19 AM <toots@rastageeks.org> wrote: >>> >>> From: Romain Beauxis <toots@rastageeks.org> >>> >>> Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata > streams >>> in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), > AV_CODEC_ID_SMPTE_KLV >>> was the only existing codec for metadata. >>> >>> It seems that this codec has a 5-bytes metadata header[1] that, for some > reason, >>> was always skipped when decoding data packets. >>> >>> However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results > in the >>> 5 first bytes of the payload being cut-off, which includes essential > informations >>> such as the ID3 tag version. >>> >>> This patch fixes the issue by keeping the 5-bytes skip only for > AV_CODEC_ID_SMPTE_KLV >>> streams. >>> >>> To test: >>> 1. download this file: > https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 >>> >>> This file was download from: > http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 >>> >>> 2. run this command: >>> ffprobe -show_streams -select_streams 0 -show_packets > -show_private_data \ >>> -show_data /path/to/bla.ts >>> >>> Before: >>> [PACKET] >>> codec_type=data >>> stream_index=0 >>> pts=494646418 >>> pts_time=5496.071311 >>> dts=494646418 >>> dts_time=5496.071311 >>> duration=N/A >>> duration_time=N/A >>> size=21 >>> pos=482784 >>> flags=K__ >>> data= >>> 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... >>> 00000010: 7465 7374 00 test. >>> >>> After: >>> [PACKET] >>> codec_type=data >>> stream_index=0 >>> pts=494646418 >>> pts_time=5496.071311 >>> dts=494646418 >>> dts_time=5496.071311 >>> duration=N/A >>> duration_time=N/A >>> size=26 >>> pos=482784 >>> flags=K__ >>> data= >>> 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. >>> 00000010: 0006 0000 0374 6573 7400 .....test. >>> >>> --- >>> libavformat/mpegts.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c >>> index fb8b0bf8fd..0b3edda817 100644 >>> --- a/libavformat/mpegts.c >>> +++ b/libavformat/mpegts.c >>> @@ -1305,7 +1305,7 @@ skip: >>> p += sl_header_bytes; >>> buf_size -= sl_header_bytes; >>> } >>> - if (pes->stream_type == 0x15 && buf_size >= 5) { >>> + if (pes->st->codecpar->codec_id == > AV_CODEC_ID_SMPTE_KLV && buf_size >= 5) { >>> /* skip metadata access unit header */ >>> pes->pes_header_size += 5; >>> p += 5; >>> -- >>> 2.39.2 (Apple Git-143) >> >> >> LGTM > > Great, thanks! > > Anything I can do to help move this to the finish line? > > Thanks, > -- Romain Pushed, thanks.
Quoting toots@rastageeks.org (2023-06-20 07:09:33) > From: Romain Beauxis <toots@rastageeks.org> > > Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata streams > in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), AV_CODEC_ID_SMPTE_KLV > was the only existing codec for metadata. > > It seems that this codec has a 5-bytes metadata header[1] that, for some reason, > was always skipped when decoding data packets. > > However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results in the > 5 first bytes of the payload being cut-off, which includes essential informations > such as the ID3 tag version. > > This patch fixes the issue by keeping the 5-bytes skip only for AV_CODEC_ID_SMPTE_KLV > streams. > > To test: > 1. download this file: https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 > > This file was download from: http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 > > 2. run this command: > ffprobe -show_streams -select_streams 0 -show_packets -show_private_data \ > -show_data /path/to/bla.ts > > Before: > [PACKET] > codec_type=data > stream_index=0 > pts=494646418 > pts_time=5496.071311 > dts=494646418 > dts_time=5496.071311 > duration=N/A > duration_time=N/A > size=21 > pos=482784 > flags=K__ > data= > 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... > 00000010: 7465 7374 00 test. > > After: > [PACKET] > codec_type=data > stream_index=0 > pts=494646418 > pts_time=5496.071311 > dts=494646418 > dts_time=5496.071311 > duration=N/A > duration_time=N/A > size=26 > pos=482784 > flags=K__ > data= > 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. > 00000010: 0006 0000 0374 6573 7400 .....test. A FATE test for this would be nice.
Le mer. 21 juin 2023 à 03:32, Anton Khirnov <anton@khirnov.net> a écrit : > > Quoting toots@rastageeks.org (2023-06-20 07:09:33) > > From: Romain Beauxis <toots@rastageeks.org> > > > > Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata streams > > in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), AV_CODEC_ID_SMPTE_KLV > > was the only existing codec for metadata. > > > > It seems that this codec has a 5-bytes metadata header[1] that, for some reason, > > was always skipped when decoding data packets. > > > > However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results in the > > 5 first bytes of the payload being cut-off, which includes essential informations > > such as the ID3 tag version. > > > > This patch fixes the issue by keeping the 5-bytes skip only for AV_CODEC_ID_SMPTE_KLV > > streams. > > > > To test: > > 1. download this file: https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 > > > > This file was download from: http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 > > > > 2. run this command: > > ffprobe -show_streams -select_streams 0 -show_packets -show_private_data \ > > -show_data /path/to/bla.ts > > > > Before: > > [PACKET] > > codec_type=data > > stream_index=0 > > pts=494646418 > > pts_time=5496.071311 > > dts=494646418 > > dts_time=5496.071311 > > duration=N/A > > duration_time=N/A > > size=21 > > pos=482784 > > flags=K__ > > data= > > 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... > > 00000010: 7465 7374 00 test. > > > > After: > > [PACKET] > > codec_type=data > > stream_index=0 > > pts=494646418 > > pts_time=5496.071311 > > dts=494646418 > > dts_time=5496.071311 > > duration=N/A > > duration_time=N/A > > size=26 > > pos=482784 > > flags=K__ > > data= > > 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. > > 00000010: 0006 0000 0374 6573 7400 .....test. > > A FATE test for this would be nice. Here's a patch attached, along with the sample that should go into fate-suite/mpegts/id3.ts The output has been verified with a local ffprobe build: packet|codec_type=data|stream_index=0|pts=126000|pts_time=1.400000|dts=126000|dts_time=1.400000|duration=N/A|duration_time=N/A|size=26|pos=564|flags=K__|data=\n00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2..\n00000010: 0006 0000 0374 6573 7400 .....test.\n|side_data|side_data_type=MPEGTS Stream ID|id=189 packet|codec_type=data|stream_index=0|pts=577350|pts_time=6.415000|dts=577350|dts_time=6.415000|duration=N/A|duration_time=N/A|size=26|pos=1316|flags=K__|data=\n00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2..\n00000010: 0006 0000 0374 6573 7400 .....test.\n|side_data|side_data_type=MPEGTS Stream ID|id=189 stream|index=0|codec_name=timed_id3|profile=unknown|codec_type=data|codec_tag_string=ID3 |codec_tag=0x20334449|ts_packetsize=188|id=0x100|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/90000|start_pts=126000|start_time=1.400000|duration_ts=451350|duration=5.015000|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=2|extradata=\n|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 format|filename=id3.ts|nb_streams=1|nb_programs=1|format_name=mpegts|start_time=1.400000|duration=5.015000|size=1504|bit_rate=2399|probe_score=2 > -- > Anton Khirnov
Quoting Romain Beauxis (2023-06-22 16:19:36) > commit ca0472eeebe478b7eb6e7d1dc4351037f8811728 > Author: Romain Beauxis <toots@rastageeks.org> > Date: Thu Jun 22 09:14:18 2023 -0500 > > Add FATE test for timed id3 demux. > > diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak > index d8fc68af88..ace8fa0b52 100644 > --- a/tests/fate/demux.mak > +++ b/tests/fate/demux.mak > @@ -157,6 +157,9 @@ fate-xwma-demux: CMD = crc -i $(TARGET_SAMPLES)/xwma/ergon.xwma -c:a copy > FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-demux > fate-ts-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/ac3/mp3ac325-4864-small.ts > > +FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-timed-id3-demux > +fate-ts-timed-id3-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/mpegts/id3.ts > + > FATE_SAMPLES_DEMUX += $(FATE_SAMPLES_DEMUX-yes) > FATE_SAMPLES_FFMPEG += $(FATE_SAMPLES_DEMUX) > FATE_FFPROBE_DEMUX += $(FATE_FFPROBE_DEMUX-yes) > diff --git a/tests/ref/fate/ts-timed-id3-demux b/tests/ref/fate/ts-timed-id3-demux > new file mode 100644 > index 0000000000..922ca17d15 > --- /dev/null > +++ b/tests/ref/fate/ts-timed-id3-demux > @@ -0,0 +1,6 @@ > +packet|codec_type=data|stream_index=0|pts=126000|pts_time=1.400000|dts=126000|dts_time=1.400000|duration=N/A|duration_time=N/A|size=26|pos=564|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 > + > +packet|codec_type=data|stream_index=0|pts=577350|pts_time=6.415000|dts=577350|dts_time=6.415000|duration=N/A|duration_time=N/A|size=26|pos=1316|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 > + > +stream|index=0|codec_name=timed_id3|profile=unknown|codec_type=data|codec_tag_string=ID3 |codec_tag=0x20334449|ts_packetsize=188|id=0x100|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/90000|start_pts=126000|start_time=1.400000|duration_ts=451350|duration=5.015000|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=2|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 > +format|filename=id3.ts|nb_streams=1|nb_programs=1|format_name=mpegts|start_time=1.400000|duration=5.015000|size=1504|bit_rate=2399|probe_score=2 Looks good, can someone please put the sample in place?
Am 24.06.23 um 12:43 schrieb Anton Khirnov: > Quoting Romain Beauxis (2023-06-22 16:19:36) >> commit ca0472eeebe478b7eb6e7d1dc4351037f8811728 >> Author: Romain Beauxis <toots@rastageeks.org> >> Date: Thu Jun 22 09:14:18 2023 -0500 >> >> Add FATE test for timed id3 demux. >> >> diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak >> index d8fc68af88..ace8fa0b52 100644 >> --- a/tests/fate/demux.mak >> +++ b/tests/fate/demux.mak >> @@ -157,6 +157,9 @@ fate-xwma-demux: CMD = crc -i $(TARGET_SAMPLES)/xwma/ergon.xwma -c:a copy >> FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-demux >> fate-ts-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/ac3/mp3ac325-4864-small.ts >> >> +FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-timed-id3-demux >> +fate-ts-timed-id3-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/mpegts/id3.ts >> + >> FATE_SAMPLES_DEMUX += $(FATE_SAMPLES_DEMUX-yes) >> FATE_SAMPLES_FFMPEG += $(FATE_SAMPLES_DEMUX) >> FATE_FFPROBE_DEMUX += $(FATE_FFPROBE_DEMUX-yes) >> diff --git a/tests/ref/fate/ts-timed-id3-demux b/tests/ref/fate/ts-timed-id3-demux >> new file mode 100644 >> index 0000000000..922ca17d15 >> --- /dev/null >> +++ b/tests/ref/fate/ts-timed-id3-demux >> @@ -0,0 +1,6 @@ >> +packet|codec_type=data|stream_index=0|pts=126000|pts_time=1.400000|dts=126000|dts_time=1.400000|duration=N/A|duration_time=N/A|size=26|pos=564|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 >> + >> +packet|codec_type=data|stream_index=0|pts=577350|pts_time=6.415000|dts=577350|dts_time=6.415000|duration=N/A|duration_time=N/A|size=26|pos=1316|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 >> + >> +stream|index=0|codec_name=timed_id3|profile=unknown|codec_type=data|codec_tag_string=ID3 |codec_tag=0x20334449|ts_packetsize=188|id=0x100|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/90000|start_pts=126000|start_time=1.400000|duration_ts=451350|duration=5.015000|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=2|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 >> +format|filename=id3.ts|nb_streams=1|nb_programs=1|format_name=mpegts|start_time=1.400000|duration=5.015000|size=1504|bit_rate=2399|probe_score=2 > > Looks good, can someone please put the sample in place? Done. 4200d5ad77c2495255742248cbfd69c3 fate-suite/mpegts/id3.ts -Thilo
Le sam. 24 juin 2023 à 05:51, Thilo Borgmann <thilo.borgmann@mail.de> a écrit : > > Am 24.06.23 um 12:43 schrieb Anton Khirnov: > > Quoting Romain Beauxis (2023-06-22 16:19:36) > >> commit ca0472eeebe478b7eb6e7d1dc4351037f8811728 > >> Author: Romain Beauxis <toots@rastageeks.org> > >> Date: Thu Jun 22 09:14:18 2023 -0500 > >> > >> Add FATE test for timed id3 demux. > >> > >> diff --git a/tests/fate/demux.mak b/tests/fate/demux.mak > >> index d8fc68af88..ace8fa0b52 100644 > >> --- a/tests/fate/demux.mak > >> +++ b/tests/fate/demux.mak > >> @@ -157,6 +157,9 @@ fate-xwma-demux: CMD = crc -i $(TARGET_SAMPLES)/xwma/ergon.xwma -c:a copy > >> FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-demux > >> fate-ts-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/ac3/mp3ac325-4864-small.ts > >> > >> +FATE_FFPROBE_DEMUX-$(CONFIG_MPEGTS_DEMUXER) += fate-ts-timed-id3-demux > >> +fate-ts-timed-id3-demux: CMD = ffprobe_demux $(TARGET_SAMPLES)/mpegts/id3.ts > >> + > >> FATE_SAMPLES_DEMUX += $(FATE_SAMPLES_DEMUX-yes) > >> FATE_SAMPLES_FFMPEG += $(FATE_SAMPLES_DEMUX) > >> FATE_FFPROBE_DEMUX += $(FATE_FFPROBE_DEMUX-yes) > >> diff --git a/tests/ref/fate/ts-timed-id3-demux b/tests/ref/fate/ts-timed-id3-demux > >> new file mode 100644 > >> index 0000000000..922ca17d15 > >> --- /dev/null > >> +++ b/tests/ref/fate/ts-timed-id3-demux > >> @@ -0,0 +1,6 @@ > >> +packet|codec_type=data|stream_index=0|pts=126000|pts_time=1.400000|dts=126000|dts_time=1.400000|duration=N/A|duration_time=N/A|size=26|pos=564|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 > >> + > >> +packet|codec_type=data|stream_index=0|pts=577350|pts_time=6.415000|dts=577350|dts_time=6.415000|duration=N/A|duration_time=N/A|size=26|pos=1316|flags=K__|data_hash=CRC32:469f474b|side_data|side_data_type=MPEGTS Stream ID|id=189 > >> + > >> +stream|index=0|codec_name=timed_id3|profile=unknown|codec_type=data|codec_tag_string=ID3 |codec_tag=0x20334449|ts_packetsize=188|id=0x100|r_frame_rate=0/0|avg_frame_rate=0/0|time_base=1/90000|start_pts=126000|start_time=1.400000|duration_ts=451350|duration=5.015000|bit_rate=N/A|max_bit_rate=N/A|bits_per_raw_sample=N/A|nb_frames=N/A|nb_read_frames=N/A|nb_read_packets=2|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 > >> +format|filename=id3.ts|nb_streams=1|nb_programs=1|format_name=mpegts|start_time=1.400000|duration=5.015000|size=1504|bit_rate=2399|probe_score=2 > > > > Looks good, can someone please put the sample in place? > > Done. > > 4200d5ad77c2495255742248cbfd69c3 fate-suite/mpegts/id3.ts Thanks! I guess we still need someone's help pushing the patch adding the test to the main repo. > -Thilo > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c index fb8b0bf8fd..0b3edda817 100644 --- a/libavformat/mpegts.c +++ b/libavformat/mpegts.c @@ -1305,7 +1305,7 @@ skip: p += sl_header_bytes; buf_size -= sl_header_bytes; } - if (pes->stream_type == 0x15 && buf_size >= 5) { + if (pes->st->codecpar->codec_id == AV_CODEC_ID_SMPTE_KLV && buf_size >= 5) { /* skip metadata access unit header */ pes->pes_header_size += 5; p += 5;
From: Romain Beauxis <toots@rastageeks.org> Before the introduction of AV_CODEC_ID_TIMED_ID3 for timed_id3 metadata streams in mpegts (commit 4a4437c0fbc8f7afe0c533070395a42e56b4ee75), AV_CODEC_ID_SMPTE_KLV was the only existing codec for metadata. It seems that this codec has a 5-bytes metadata header[1] that, for some reason, was always skipped when decoding data packets. However, when working with a AV_CODEC_ID_TIMED_ID3 streams, this results in the 5 first bytes of the payload being cut-off, which includes essential informations such as the ID3 tag version. This patch fixes the issue by keeping the 5-bytes skip only for AV_CODEC_ID_SMPTE_KLV streams. To test: 1. download this file: https://www.dropbox.com/s/jy8sih3pe8qskxb/bla.ts?dl=1 This file was download from: http://playertest.longtailvideo.com/adaptive/wowzaid3/playlist.m3u8 2. run this command: ffprobe -show_streams -select_streams 0 -show_packets -show_private_data \ -show_data /path/to/bla.ts Before: [PACKET] codec_type=data stream_index=0 pts=494646418 pts_time=5496.071311 dts=494646418 dts_time=5496.071311 duration=N/A duration_time=N/A size=21 pos=482784 flags=K__ data= 00000000: 0000 0000 1054 4954 3200 0000 0600 0003 .....TIT2....... 00000010: 7465 7374 00 test. After: [PACKET] codec_type=data stream_index=0 pts=494646418 pts_time=5496.071311 dts=494646418 dts_time=5496.071311 duration=N/A duration_time=N/A size=26 pos=482784 flags=K__ data= 00000000: 4944 3304 0000 0000 0010 5449 5432 0000 ID3.......TIT2.. 00000010: 0006 0000 0374 6573 7400 .....test. --- libavformat/mpegts.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)