Re: [FFmpeg-devel] [PATCH 17/28] ffmpeg: move the mux queue into muxer private data

[Anton Khirnov <anton@khirnov.net>]

Quoting Andreas Rheinhardt (2022-01-13 11:50:48)
> 
> My objections to adding a separately allocated muxing context and to
> this MuxStream have not changed. Both incur unnecessary allocations
> and indirections and (in case of the latter) loops;

I cannot imagine any remotely real situation where the cost of these
allocation and loops is anywhere close to being meaningful. So your
argument amounts to vastly premature optimization.

> the latter is also very unnatural. The patch here actually shows it:
> You only use the muxer context to get the MuxStream context
> corresponding to the OutputStream you are interested in:

I don't see how that shows any unnaturalness. As I said last time
already - the idea is to (eventually) split OutputStream into separate
encoding and muxing objects that can be used independently.

If you really insist I can wait until actually implementing that split,
but I'd rather avoid giant branches if possible.

> 
> >      for (i = 0; i < of->ctx->nb_streams; i++) {
> > +        MuxStream     *ms = &of->mux->streams[i];
> >          OutputStream *ost = output_streams[of->ost_index + i];
> 
> >      AVStream *st = ost->st;
> > +    MuxStream *ms = &of->mux->streams[st->index];
> >      int ret;
> 
> Your aim of making sure what code can use/modify what parts can also be
> fulfilled by comments.

In my experience that simply does not work. People either don't read the
comments at all or read them once and forget. Did you see what happened
to all those fields in AVStream marked as "libavformat private use only"?

-- 
Anton Khirnov
_______________________________________________
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".