| Message ID | 66947c1870e54fdf87b2c0c06495c3ec@scisys.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Dec 09, 2019 at 10:22:15 +0000, Jörg Beckmann wrote: Just some formal stuff: > Subject: Patch for memory optimization with QuickTime/MP4 This subject should be created by the actual patch, because, they way you submitted it, it will be used for pushing. Or you create your patch with "git format-patch" and attach it, then the actual subject of the e-mail doesn't matter. Or "git send-email" will format the subject for you, assuming you put the correct text into your commit message: That said, the commit message should be introduced with a one-liner with a module prefix, such as: avformat/mov: memory optimization with QuickTime/MP4 (I think it should be "for", not "with", but that's up to you in the first step.) Then an empty line, then the remaining text. No need to mention "patch" anywhere in the commit message, because it's obvious that a commit corresponds to a patch. > The last patch mail seems to be lost in space. Therefore here the next try . > > Cheers, > Jörg If you write this text here, it will be part of the pushed commit message. If you wish to add text to a patch sent with "git send-email", please add your additional text below the three-dash separator: > This patch invents a new option "discard_fragments" for the MP4/Quicktime/MOV decoder. If the option is not set, nothing changes at all. If it is set, old fragments are discarded as far as possible on each call to switch_root. For pure audio streams, the memory usage is now constant. For video streams, the memory usage is reduced. I've tested it with audio streams received from a professional DAB+ receiver and with video streams created on my own with "ffmpeg -i <video>.m4v -c:a:0 copy -c:v copy -c:s copy -f ismv -movflags frag_keyframe -movflags faststart tcp://localhost:1234?listen" and "ffmpeg -i tcp://localhost:1234 -c:a copy -c:v copy -c:s copy -y <output>". > > Signed-off-by: Jörg Beckmann <joerg.beckmann@scisys.com> > --- (Add your text in here.) You should also kindly wrap your commit message at ~80 characters, or perhaps 72. > + for (i = 0; i < mov->fc->nb_streams; i++) { > + if (mov->fc->streams[i]->id == frag->track_id) { > + st = mov->fc->streams[i]; > + break; > + } > + } > + > + av_assert0(st); This can never happen? Or can it, with an illegally formatted file, but shouldn't? In the latter case, you would need to error out with "invalid data". (Just wondering, not critisizing.) > + case AVMEDIA_TYPE_SUBTITLE: > + /* Freeing VIDEO tables leads to corrupted video when writing to eg. MKV */ > + av_freep(&st->index_entries); > + st->nb_index_entries = 0; av_freep() NULLs for you. > + av_freep(&sc->ctts_data); > + sc->ctts_data = NULL; Ditto. > + {"discard_fragments", > + "Discards fragments after they have been read to support live streams.", Nit: I think these descriptions are imperatives, so drop the 's' from "Discards". Cheers, Moritz
On Mon, Dec 09, 2019 at 13:15:40 +0100, Moritz Barsnick wrote: > > + case AVMEDIA_TYPE_SUBTITLE: > > + /* Freeing VIDEO tables leads to corrupted video when writing to eg. MKV */ > > + av_freep(&st->index_entries); > > + st->nb_index_entries = 0; > > av_freep() NULLs for you. Forget this one, I misread it. ;-) > > > + av_freep(&sc->ctts_data); > > + sc->ctts_data = NULL; > > Ditto. But my comment applies here. Moritz
> -----Ursprüngliche Nachricht----- > Von: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> Im Auftrag von Moritz > Barsnick > Gesendet: Montag, 9. Dezember 2019 13:16 > An: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Betreff: [SCISYS Possible Spam] Re: [FFmpeg-devel] [PATCH v4] Patch for > memory optimization with QuickTime/MP4 > > On Mon, Dec 09, 2019 at 10:22:15 +0000, Jörg Beckmann wrote: > > Just some formal stuff: > > > Subject: Patch for memory optimization with QuickTime/MP4 > > This subject should be created by the actual patch, because, they way you > submitted it, it will be used for pushing. > > Or you create your patch with "git format-patch" and attach it, then the actual > subject of the e-mail doesn't matter. Or "git send-email" > will format the subject for you, assuming you put the correct text into your commit > message: > > That said, the commit message should be introduced with a one-liner with a > module prefix, such as: > > avformat/mov: memory optimization with QuickTime/MP4 > > (I think it should be "for", not "with", but that's up to you in the first step.) > > Then an empty line, then the remaining text. No need to mention "patch" > anywhere in the commit message, because it's obvious that a commit corresponds > to a patch. > > > The last patch mail seems to be lost in space. Therefore here the next try . > > > > Cheers, > > Jörg > > If you write this text here, it will be part of the pushed commit message. If you wish > to add text to a patch sent with "git send-email", please add your additional text > below the three-dash separator: > > > This patch invents a new option "discard_fragments" for the > MP4/Quicktime/MOV decoder. If the option is not set, nothing changes at all. If it > is set, old fragments are discarded as far as possible on each call to switch_root. > For pure audio streams, the memory usage is now constant. For video streams, > the memory usage is reduced. I've tested it with audio streams received from a > professional DAB+ receiver and with video streams created on my own with > "ffmpeg -i <video>.m4v -c:a:0 copy -c:v copy -c:s copy -f ismv -movflags > frag_keyframe -movflags faststart tcp://localhost:1234?listen" and "ffmpeg -i > tcp://localhost:1234 -c:a copy -c:v copy -c:s copy -y <output>". > > > > Signed-off-by: Jörg Beckmann <joerg.beckmann@scisys.com> > > --- > > (Add your text in here.) > > You should also kindly wrap your commit message at ~80 characters, or perhaps > 72. Okay, I'll send it again. > > > + for (i = 0; i < mov->fc->nb_streams; i++) { > > + if (mov->fc->streams[i]->id == frag->track_id) { > > + st = mov->fc->streams[i]; > > + break; > > + } > > + } > > + > > + av_assert0(st); > > This can never happen? Or can it, with an illegally formatted file, but shouldn't? In > the latter case, you would need to error out with "invalid data". (Just wondering, not > critisizing.) The assert() results from a discussion with Carl Eugen Hoyos. There was an error message before, but he suggested to replace it because it should not be possible. > > + case AVMEDIA_TYPE_SUBTITLE: > > + /* Freeing VIDEO tables leads to corrupted video when writing to eg. > MKV */ > > + av_freep(&st->index_entries); > > + st->nb_index_entries = 0; > > av_freep() NULLs for you. > > > + av_freep(&sc->ctts_data); > > + sc->ctts_data = NULL; > > Ditto. Oops, I forgot to change it here. > > + {"discard_fragments", > > + "Discards fragments after they have been read to support > > + live streams.", > > Nit: I think these descriptions are imperatives, so drop the 's' from "Discards". I think you're right. > > Cheers, > Moritz > _______________________________________________ > 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/isom.h b/libavformat/isom.h index 4943b80ccf..9b4753f4d7 100644 --- a/libavformat/isom.h +++ b/libavformat/isom.h @@ -268,6 +268,7 @@ typedef struct MOVContext { int advanced_editlist; int ignore_chapters; int seek_individually; + int discard_fragments; int64_t next_root_atom; ///< offset of the next root atom int export_all; int export_xmp; diff --git a/libavformat/mov.c b/libavformat/mov.c index 7553a7fdfc..97c02725c5 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -7698,8 +7698,11 @@ static int should_retry(AVIOContext *pb, int error_code) { static int mov_switch_root(AVFormatContext *s, int64_t target, int index) { - int ret; + int ret, i; MOVContext *mov = s->priv_data; + AVStream *st = NULL; + MOVStreamContext *sc; + MOVFragment *frag; if (index >= 0 && index < mov->frag_index.nb_items) target = mov->frag_index.item[index].moof_offset; @@ -7721,6 +7724,44 @@ static int mov_switch_root(AVFormatContext *s, int64_t target, int index) mov->found_mdat = 0; + if (mov->discard_fragments) { + frag = &mov->fragment; + + for (i = 0; i < mov->fc->nb_streams; i++) { + if (mov->fc->streams[i]->id == frag->track_id) { + st = mov->fc->streams[i]; + break; + } + } + + av_assert0(st); + + sc = st->priv_data; + + switch (st->codecpar->codec_type) { + case AVMEDIA_TYPE_AUDIO: + case AVMEDIA_TYPE_SUBTITLE: + /* Freeing VIDEO tables leads to corrupted video when writing to eg. MKV */ + av_freep(&st->index_entries); + st->nb_index_entries = 0; + st->index_entries_allocated_size = 0; + + sc->current_index = 0; + sc->current_sample = 0; + + av_freep(&sc->ctts_data); + sc->ctts_data = NULL; + sc->ctts_allocated_size = 0; + sc->ctts_count = 0; + break; + } + + av_free(mov->frag_index.item->stream_info); + av_freep(&mov->frag_index.item); + mov->frag_index.allocated_size = 0; + mov->frag_index.nb_items = 0; + } + ret = mov_read_default(mov, s->pb, (MOVAtom){ AV_RL32("root"), INT64_MAX }); if (ret < 0) return ret; @@ -7975,6 +8016,9 @@ static int mov_read_seek(AVFormatContext *s, int stream_index, int64_t sample_ti int sample; int i; + if (mc->discard_fragments) // Seeking is not possible if fragments are discarded. + return AVERROR(ENOTSUP); + if (stream_index >= s->nb_streams) return AVERROR_INVALIDDATA; @@ -8063,6 +8107,10 @@ static const AVOption mov_options[] = { { "decryption_key", "The media decryption key (hex)", OFFSET(decryption_key), AV_OPT_TYPE_BINARY, .flags = AV_OPT_FLAG_DECODING_PARAM }, { "enable_drefs", "Enable external track support.", OFFSET(enable_drefs), AV_OPT_TYPE_BOOL, {.i64 = 0}, 0, 1, FLAGS }, + {"discard_fragments", + "Discards fragments after they have been read to support live streams.", + OFFSET(discard_fragments), AV_OPT_TYPE_BOOL, { .i64 = 0 }, + 0, 1, FLAGS }, { NULL }, };
The last patch mail seems to be lost in space. Therefore here the next try . Cheers, Jörg This patch invents a new option "discard_fragments" for the MP4/Quicktime/MOV decoder. If the option is not set, nothing changes at all. If it is set, old fragments are discarded as far as possible on each call to switch_root. For pure audio streams, the memory usage is now constant. For video streams, the memory usage is reduced. I've tested it with audio streams received from a professional DAB+ receiver and with video streams created on my own with "ffmpeg -i <video>.m4v -c:a:0 copy -c:v copy -c:s copy -f ismv -movflags frag_keyframe -movflags faststart tcp://localhost:1234?listen" and "ffmpeg -i tcp://localhost:1234 -c:a copy -c:v copy -c:s copy -y <output>". Signed-off-by: Jörg Beckmann <joerg.beckmann@scisys.com> --- libavformat/isom.h | 1 + libavformat/mov.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 50 insertions(+), 1 deletion(-)