[FFmpeg-devel] [PATCH] fftools/ffmpeg, ffmpeg_opt: Allocate (In|Out)putStream.pkt early

[James Almer]

On 10/8/2021 8:49 AM, Andreas Rheinhardt wrote:
> Avoids checks lateron in the hot path.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
> ---
>   fftools/ffmpeg.c     | 16 +---------------
>   fftools/ffmpeg_opt.c |  4 ++--
>   2 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 14611480f1..9d4f9d7a2b 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -1533,9 +1533,6 @@ static int reap_filters(int flush)
>           if (av_buffersink_get_type(filter) == AVMEDIA_TYPE_AUDIO)
>               init_output_stream_wrapper(ost, NULL, 1);
>   
> -        if (!ost->pkt && !(ost->pkt = av_packet_alloc())) {
> -            return AVERROR(ENOMEM);
> -        }
>           if (!ost->filtered_frame && !(ost->filtered_frame = av_frame_alloc())) {
>               return AVERROR(ENOMEM);
>           }
> @@ -1979,9 +1976,6 @@ static void flush_encoders(void)
>               AVPacket *pkt = ost->pkt;
>               int pkt_size;
>   
> -            if (!pkt)
> -                break;
> -
>               switch (enc->codec_type) {
>               case AVMEDIA_TYPE_AUDIO:
>                   desc   = "audio";
> @@ -2597,8 +2591,6 @@ static int transcode_subtitles(InputStream *ist, AVPacket *pkt, int *got_output,
>       for (i = 0; i < nb_output_streams; i++) {
>           OutputStream *ost = output_streams[i];
>   
> -        if (!ost->pkt && !(ost->pkt = av_packet_alloc()))
> -            exit_program(1);
>           if (!check_output_constraints(ist, ost) || !ost->encoding_needed
>               || ost->enc->type != AVMEDIA_TYPE_SUBTITLE)
>               continue;
> @@ -2634,11 +2626,7 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>       int repeating = 0;
>       int eof_reached = 0;
>   
> -    AVPacket *avpkt;
> -
> -    if (!ist->pkt && !(ist->pkt = av_packet_alloc()))
> -        return AVERROR(ENOMEM);
> -    avpkt = ist->pkt;
> +    AVPacket *avpkt = ist->pkt;
>   
>       if (!ist->saw_first_ts) {
>           ist->first_dts =
> @@ -2809,8 +2797,6 @@ static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eo
>       for (i = 0; i < nb_output_streams; i++) {
>           OutputStream *ost = output_streams[i];
>   
> -        if (!ost->pkt && !(ost->pkt = av_packet_alloc()))
> -            exit_program(1);
>           if (!check_output_constraints(ist, ost) || ost->encoding_needed)
>               continue;
>   
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index c46a9343e1..e99967ebb3 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -824,7 +824,7 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic)
>           const AVOption *discard_opt = av_opt_find(&cc, "skip_frame", NULL,
>                                                     0, AV_OPT_SEARCH_FAKE_OBJ);
>   
> -        if (!ist)
> +        if (!ist || !(ist->pkt = av_packet_alloc()))

I'd prefer to do it lower in the function, next to where ist->dec_ctx is 
allocated. Keep checks and allocations clearly split and cleaner looking.

>               exit_program(1);
>   
>           GROW_ARRAY(input_streams, nb_input_streams);
> @@ -1486,7 +1486,7 @@ static OutputStream *new_output_stream(OptionsContext *o, AVFormatContext *oc, e
>           st->id = o->streamid_map[oc->nb_streams - 1];
>   
>       GROW_ARRAY(output_streams, nb_output_streams);
> -    if (!(ost = av_mallocz(sizeof(*ost))))
> +    if (!(ost = av_mallocz(sizeof(*ost))) || !(ost->pkt = av_packet_alloc()))

Same.

>           exit_program(1);
>       output_streams[nb_output_streams - 1] = ost;

LGTM otherwise.