| Message ID | M610EAj--3-2@lynne.ee |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel] oggdec: add support for proper demuxing of chained Opus files and streams | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/configure | warning | Failed to apply patch |
Am Di., 28. Apr. 2020 um 20:20 Uhr schrieb Lynne <dev@lynne.ee>: > > Part of this patch is based on Paul B Mahol's patch from last year. > > This also allows for single-stream parameter/codec changes. > > Must be applied on top of the latest version of other 3 patches I sent today. Are they related to tickets #868 or #7011? Carl Eugen
Apr 29, 2020, 00:02 by ceffmpeg@gmail.com: > Am Di., 28. Apr. 2020 um 20:20 Uhr schrieb Lynne <dev@lynne.ee>: > >> >> Part of this patch is based on Paul B Mahol's patch from last year. >> >> This also allows for single-stream parameter/codec changes. >> >> Must be applied on top of the latest version of other 3 patches I sent today. >> > > Are they related to tickets #868 or #7011? > Only to 7011. Will close it once this is in master.
On Tue, Apr 28, 2020 at 08:20:37PM +0200, Lynne wrote: > Part of this patch is based on Paul B Mahol's patch from last year. > > This also allows for single-stream parameter/codec changes. > > Must be applied on top of the latest version of other 3 patches I sent today. > > oggdec.c | 45 +++++++++++++++++++++++++-------------------- > oggdec.h | 1 + > oggparseopus.c | 1 + > 3 files changed, 27 insertions(+), 20 deletions(-) > ce692abc11552b4c35772e57051378e0fd1ddece 0001-oggdec-add-support-for-proper-demuxing-of-chained-Op.patch > From 70dcc91b32c89cb580bf13f2c081fa8e74f226f9 Mon Sep 17 00:00:00 2001 > From: Lynne <dev@lynne.ee> > Date: Tue, 28 Apr 2020 12:25:46 +0100 > Subject: [PATCH] oggdec: add support for proper demuxing of chained Opus files > and streams > > Part of this patch is based on Paul B Mahol's patch from last year. > > This also allows for single-stream parameter/codec changes. > --- > libavformat/oggdec.c | 45 +++++++++++++++++++++----------------- > libavformat/oggdec.h | 1 + > libavformat/oggparseopus.c | 1 + > 3 files changed, 27 insertions(+), 20 deletions(-) This causes out of array reads with https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg ==5283== Invalid read of size 8 ==5283== at 0x640508: vorbis_packet (oggparsevorbis.c:413) ==5283== by 0x637546: ogg_packet (oggdec.c:589) ==5283== by 0x638392: ogg_read_packet (oggdec.c:824) ==5283== by 0x6A9211: ff_read_packet (utils.c:851) ==5283== by 0x6AC440: read_frame_internal (utils.c:1582) ==5283== by 0x6AD3F8: av_read_frame (utils.c:1784) ==5283== by 0x250B4B: get_input_packet (ffmpeg.c:4140) ==5283== by 0x251021: process_input (ffmpeg.c:4259) ==5283== by 0x253255: transcode_step (ffmpeg.c:4640) ==5283== by 0x2533D2: transcode (ffmpeg.c:4694) ==5283== by 0x253CE9: main (ffmpeg.c:4895) ==5283== Address 0x1680af68 is 8 bytes after a block of size 32 in arena "client" ==5283== [...]
On Mon, Jun 01, 2020 at 06:19:52PM +0200, Michael Niedermayer wrote: > On Tue, Apr 28, 2020 at 08:20:37PM +0200, Lynne wrote: > > Part of this patch is based on Paul B Mahol's patch from last year. > > > > This also allows for single-stream parameter/codec changes. > > > > Must be applied on top of the latest version of other 3 patches I sent today. > > > > > oggdec.c | 45 +++++++++++++++++++++++++-------------------- > > oggdec.h | 1 + > > oggparseopus.c | 1 + > > 3 files changed, 27 insertions(+), 20 deletions(-) > > ce692abc11552b4c35772e57051378e0fd1ddece 0001-oggdec-add-support-for-proper-demuxing-of-chained-Op.patch > > From 70dcc91b32c89cb580bf13f2c081fa8e74f226f9 Mon Sep 17 00:00:00 2001 > > From: Lynne <dev@lynne.ee> > > Date: Tue, 28 Apr 2020 12:25:46 +0100 > > Subject: [PATCH] oggdec: add support for proper demuxing of chained Opus files > > and streams > > > > Part of this patch is based on Paul B Mahol's patch from last year. > > > > This also allows for single-stream parameter/codec changes. > > --- > > libavformat/oggdec.c | 45 +++++++++++++++++++++----------------- > > libavformat/oggdec.h | 1 + > > libavformat/oggparseopus.c | 1 + > > 3 files changed, 27 insertions(+), 20 deletions(-) > > This causes out of array reads with > https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg > > ==5283== Invalid read of size 8 > ==5283== at 0x640508: vorbis_packet (oggparsevorbis.c:413) > ==5283== by 0x637546: ogg_packet (oggdec.c:589) > ==5283== by 0x638392: ogg_read_packet (oggdec.c:824) > ==5283== by 0x6A9211: ff_read_packet (utils.c:851) > ==5283== by 0x6AC440: read_frame_internal (utils.c:1582) > ==5283== by 0x6AD3F8: av_read_frame (utils.c:1784) > ==5283== by 0x250B4B: get_input_packet (ffmpeg.c:4140) > ==5283== by 0x251021: process_input (ffmpeg.c:4259) > ==5283== by 0x253255: transcode_step (ffmpeg.c:4640) > ==5283== by 0x2533D2: transcode (ffmpeg.c:4694) > ==5283== by 0x253CE9: main (ffmpeg.c:4895) > ==5283== Address 0x1680af68 is 8 bytes after a block of size 32 in arena "client" > ==5283== ping [...]
Jun 6, 2020, 17:21 by michael@niedermayer.cc: > On Mon, Jun 01, 2020 at 06:19:52PM +0200, Michael Niedermayer wrote: > >> On Tue, Apr 28, 2020 at 08:20:37PM +0200, Lynne wrote: >> > Part of this patch is based on Paul B Mahol's patch from last year. >> > >> > This also allows for single-stream parameter/codec changes. >> > >> > Must be applied on top of the latest version of other 3 patches I sent today. >> > >> >> > oggdec.c | 45 +++++++++++++++++++++++++-------------------- >> > oggdec.h | 1 + >> > oggparseopus.c | 1 + >> > 3 files changed, 27 insertions(+), 20 deletions(-) >> > ce692abc11552b4c35772e57051378e0fd1ddece 0001-oggdec-add-support-for-proper-demuxing-of-chained-Op.patch >> > From 70dcc91b32c89cb580bf13f2c081fa8e74f226f9 Mon Sep 17 00:00:00 2001 >> > From: Lynne <dev@lynne.ee> >> > Date: Tue, 28 Apr 2020 12:25:46 +0100 >> > Subject: [PATCH] oggdec: add support for proper demuxing of chained Opus files >> > and streams >> > >> > Part of this patch is based on Paul B Mahol's patch from last year. >> > >> > This also allows for single-stream parameter/codec changes. >> > --- >> > libavformat/oggdec.c | 45 +++++++++++++++++++++----------------- >> > libavformat/oggdec.h | 1 + >> > libavformat/oggparseopus.c | 1 + >> > 3 files changed, 27 insertions(+), 20 deletions(-) >> >> This causes out of array reads with >> https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg >> >> ==5283== Invalid read of size 8 >> ==5283== at 0x640508: vorbis_packet (oggparsevorbis.c:413) >> ==5283== by 0x637546: ogg_packet (oggdec.c:589) >> ==5283== by 0x638392: ogg_read_packet (oggdec.c:824) >> ==5283== by 0x6A9211: ff_read_packet (utils.c:851) >> ==5283== by 0x6AC440: read_frame_internal (utils.c:1582) >> ==5283== by 0x6AD3F8: av_read_frame (utils.c:1784) >> ==5283== by 0x250B4B: get_input_packet (ffmpeg.c:4140) >> ==5283== by 0x251021: process_input (ffmpeg.c:4259) >> ==5283== by 0x253255: transcode_step (ffmpeg.c:4640) >> ==5283== by 0x2533D2: transcode (ffmpeg.c:4694) >> ==5283== by 0x253CE9: main (ffmpeg.c:4895) >> ==5283== Address 0x1680af68 is 8 bytes after a block of size 32 in arena "client" >> ==5283== >> > > ping > Not sure how that's possible. The codec-specific parsing context just disappears?
On Sat, Jun 06, 2020 at 07:23:25PM +0200, Lynne wrote: > Jun 6, 2020, 17:21 by michael@niedermayer.cc: > > > On Mon, Jun 01, 2020 at 06:19:52PM +0200, Michael Niedermayer wrote: > > > >> On Tue, Apr 28, 2020 at 08:20:37PM +0200, Lynne wrote: > >> > Part of this patch is based on Paul B Mahol's patch from last year. > >> > > >> > This also allows for single-stream parameter/codec changes. > >> > > >> > Must be applied on top of the latest version of other 3 patches I sent today. > >> > > >> > >> > oggdec.c | 45 +++++++++++++++++++++++++-------------------- > >> > oggdec.h | 1 + > >> > oggparseopus.c | 1 + > >> > 3 files changed, 27 insertions(+), 20 deletions(-) > >> > ce692abc11552b4c35772e57051378e0fd1ddece 0001-oggdec-add-support-for-proper-demuxing-of-chained-Op.patch > >> > From 70dcc91b32c89cb580bf13f2c081fa8e74f226f9 Mon Sep 17 00:00:00 2001 > >> > From: Lynne <dev@lynne.ee> > >> > Date: Tue, 28 Apr 2020 12:25:46 +0100 > >> > Subject: [PATCH] oggdec: add support for proper demuxing of chained Opus files > >> > and streams > >> > > >> > Part of this patch is based on Paul B Mahol's patch from last year. > >> > > >> > This also allows for single-stream parameter/codec changes. > >> > --- > >> > libavformat/oggdec.c | 45 +++++++++++++++++++++----------------- > >> > libavformat/oggdec.h | 1 + > >> > libavformat/oggparseopus.c | 1 + > >> > 3 files changed, 27 insertions(+), 20 deletions(-) > >> > >> This causes out of array reads with > >> https://samples.ffmpeg.org/V-codecs/Theora/theora_testsuite_broken/multi2.ogg > >> > >> ==5283== Invalid read of size 8 > >> ==5283== at 0x640508: vorbis_packet (oggparsevorbis.c:413) > >> ==5283== by 0x637546: ogg_packet (oggdec.c:589) > >> ==5283== by 0x638392: ogg_read_packet (oggdec.c:824) > >> ==5283== by 0x6A9211: ff_read_packet (utils.c:851) > >> ==5283== by 0x6AC440: read_frame_internal (utils.c:1582) > >> ==5283== by 0x6AD3F8: av_read_frame (utils.c:1784) > >> ==5283== by 0x250B4B: get_input_packet (ffmpeg.c:4140) > >> ==5283== by 0x251021: process_input (ffmpeg.c:4259) > >> ==5283== by 0x253255: transcode_step (ffmpeg.c:4640) > >> ==5283== by 0x2533D2: transcode (ffmpeg.c:4694) > >> ==5283== by 0x253CE9: main (ffmpeg.c:4895) > >> ==5283== Address 0x1680af68 is 8 bytes after a block of size 32 in arena "client" > >> ==5283== > >> > > > > ping > > > > Not sure how that's possible. The codec-specific parsing context just disappears? i have a few more crashes from this patchset and looked at the others first, i will post fixes to 2 of them. This one here i did not deeply look at yet, so i cant say what is happening yet ... thx [...]
From 70dcc91b32c89cb580bf13f2c081fa8e74f226f9 Mon Sep 17 00:00:00 2001 From: Lynne <dev@lynne.ee> Date: Tue, 28 Apr 2020 12:25:46 +0100 Subject: [PATCH] oggdec: add support for proper demuxing of chained Opus files and streams Part of this patch is based on Paul B Mahol's patch from last year. This also allows for single-stream parameter/codec changes. --- libavformat/oggdec.c | 45 +++++++++++++++++++++----------------- libavformat/oggdec.h | 1 + libavformat/oggparseopus.c | 1 + 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c index 92dcafe2ed..c591bafddd 100644 --- a/libavformat/oggdec.c +++ b/libavformat/oggdec.c @@ -178,6 +178,7 @@ static int ogg_reset(AVFormatContext *s) if (start_pos <= s->internal->data_offset) { os->lastpts = 0; } + os->start_trimming = 0; os->end_trimming = 0; av_freep(&os->new_metadata); os->new_metadata_size = 0; @@ -206,7 +207,8 @@ static const struct ogg_codec *ogg_find_codec(uint8_t *buf, int size) * situation where a new audio stream spawn (identified with a new serial) and * must replace the previous one (track switch). */ -static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic) +static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic, + int probing) { struct ogg *ogg = s->priv_data; struct ogg_stream *os; @@ -220,24 +222,25 @@ static int ogg_replace_stream(AVFormatContext *s, uint32_t serial, char *magic) /* Check for codecs */ codec = ogg_find_codec(magic, 8); - if (!codec) { + if (!codec && !probing) { av_log(s, AV_LOG_ERROR, "Cannot identify new stream\n"); return AVERROR_INVALIDDATA; } - /* If the codec matches, then we assume its a replacement */ - for (i = 0; i < ogg->nstreams; i++) { - if (ogg->streams[i].codec == codec) - break; - } - - /* Otherwise, create a new stream */ - if (i >= ogg->nstreams) - return ogg_new_stream(s, serial); - - os = &ogg->streams[i]; - os->serial = serial; - os->codec = codec; + /* We only have a single stream anyway, so if there's a new stream with + * a different codec just replace it */ + os = &ogg->streams[0]; + os->serial = serial; + os->codec = codec; + os->serial = serial; + os->lastpts = 0; + os->lastdts = 0; + os->start_trimming = 0; + os->end_trimming = 0; + + /* Chained files have extradata as a new packet */ + if (codec == &ff_opus_codec) + os->header = -1; return i; } @@ -294,7 +297,7 @@ static int data_packets_seen(const struct ogg *ogg) return 0; } -static int ogg_read_page(AVFormatContext *s, int *sid) +static int ogg_read_page(AVFormatContext *s, int *sid, int probing) { AVIOContext *bc = s->pb; struct ogg *ogg = s->priv_data; @@ -417,7 +420,7 @@ static int ogg_read_page(AVFormatContext *s, int *sid) /* CRC is correct so we can be 99% sure there's an actual change here */ if (idx < 0) { if (data_packets_seen(ogg)) - idx = ogg_replace_stream(s, serial, readout_buf); + idx = ogg_replace_stream(s, serial, readout_buf, probing); else idx = ogg_new_stream(s, serial); @@ -492,7 +495,7 @@ static int ogg_packet(AVFormatContext *s, int *sid, int *dstart, int *dsize, idx = ogg->curidx; while (idx < 0) { - ret = ogg_read_page(s, &idx); + ret = ogg_read_page(s, &idx, 0); if (ret < 0) return ret; } @@ -643,7 +646,7 @@ static int ogg_get_length(AVFormatContext *s) avio_seek(s->pb, end, SEEK_SET); ogg->page_pos = -1; - while (!ogg_read_page(s, &i)) { + while (!ogg_read_page(s, &i, 1)) { if (ogg->streams[i].granule != -1 && ogg->streams[i].granule != 0 && ogg->streams[i].codec) { s->streams[i]->duration = @@ -847,13 +850,15 @@ retry: pkt->duration = os->pduration; pkt->pos = fpos; - if (os->end_trimming) { + if (os->start_trimming || os->end_trimming) { uint8_t *side_data = av_packet_new_side_data(pkt, AV_PKT_DATA_SKIP_SAMPLES, 10); if(!side_data) return AVERROR(ENOMEM); + AV_WL32(side_data + 0, os->start_trimming); AV_WL32(side_data + 4, os->end_trimming); + os->start_trimming = 0; os->end_trimming = 0; } diff --git a/libavformat/oggdec.h b/libavformat/oggdec.h index 4a2b6ddee8..e2057c46f6 100644 --- a/libavformat/oggdec.h +++ b/libavformat/oggdec.h @@ -84,6 +84,7 @@ struct ogg_stream { int got_start; int got_data; ///< 1 if the stream got some data (non-initial packets), 0 otherwise int nb_header; ///< set to the number of parsed headers + int start_trimming; ///< set the number of packets to drop from the start int end_trimming; ///< set the number of packets to drop from the end uint8_t *new_metadata; unsigned int new_metadata_size; diff --git a/libavformat/oggparseopus.c b/libavformat/oggparseopus.c index 56b53e74e8..36d691e9aa 100644 --- a/libavformat/oggparseopus.c +++ b/libavformat/oggparseopus.c @@ -59,6 +59,7 @@ static int opus_header(AVFormatContext *avf, int idx) priv->pre_skip = AV_RL16(packet + 10); st->codecpar->initial_padding = priv->pre_skip; + os->start_trimming = priv->pre_skip; /*orig_sample_rate = AV_RL32(packet + 12);*/ /*gain = AV_RL16(packet + 16);*/ /*channel_map = AV_RL8 (packet + 18);*/ -- 2.26.2