[FFmpeg-devel] [SCISYS Possible Spam] Re: [PATCH v4] Patch for memory optimization with QuickTime/MP4

[Jörg Beckmann]

> -----Ursprüngliche Nachricht-----
> Von: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> Im Auftrag von Moritz
> Barsnick
> Gesendet: Montag, 9. Dezember 2019 13:16
> An: FFmpeg development discussions and patches <ffmpeg-devel at 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 at 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 at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request at ffmpeg.org with
> subject "unsubscribe".