From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by master.gitmailbox.com (Postfix) with ESMTP id 9BCB143E63 for ; Tue, 16 Aug 2022 12:21:55 +0000 (UTC) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id F3A6468B945; Tue, 16 Aug 2022 15:21:35 +0300 (EEST) Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id D34AE68B8EB for ; Tue, 16 Aug 2022 15:21:34 +0300 (EEST) Received: by mail-vs1-f47.google.com with SMTP id c3so9892882vsc.6 for ; Tue, 16 Aug 2022 05:21:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc; bh=Aalo5MystQddHwOeOB4EH+wfVlWwyks6pWXvsx+kSaw=; b=e4U++ClVxB2zjC/L5OAsSVtfph3yU8qnIYjC1wloE1uneHEw0HaRpUOyIV6uL/qk4Y SX84g2Ovw5T7bmFipb0pqZy3XaFPmlls3aA5Kmx1wgjO3L11dENnZEBt7o8PONwvqeYw p1nUMIjMYaKk/x/lrhJHgsiNwa7lmNTkotboR7BIcyO8Wafd8rq+/Wu7UDA0FzCc18t7 OaNgTOl+2TcCFONGZ0YyimtuEDR0VRCaE0RrqxUZCw5OWq+/Xmjv9aIeiZfYUaEhcrY3 Z7XymSg/K/Fv4cC8VlWeAA+G5+WugkeMIpudiW/hnEaPI+wZ96wHc9a6MvMKr7itBxwM /Izw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=Aalo5MystQddHwOeOB4EH+wfVlWwyks6pWXvsx+kSaw=; b=wsJ16jSNBrSd6p/Ucz7ksN076bXIzcCrW45b7+sWRc5fV2pVi4RKk6LQUpefluxPO+ f9+xgYHx3A76EJuSjCOzjU1NSR+BzyCbJCgm7gvrbbxs9maNoLO4mnSFM06bDJpdr5EP TpEQr3yszKiTRasCk2y4Q7SmAfyExz0BuW65IM8YWwnFUT64ktCPaCmpfef7tAK770Ci vCcIVid67A8272mA8nc6wC1PzjBgp1CkSpTKTYUV4SadIX/39oA35NeKAv96/6f5ZMco EVY0xYc2w7tk7TRZrMoZiOBn5JAdKubu6RwTkBUaIIe1w4Umlt0J7XzWd47JJp1am0Tp Oq9Q== X-Gm-Message-State: ACgBeo1AN57cBHXTXHnGqpQa5NmFi4oCUpSYvhC4OaJ5/6vDVAg3h8RD BhDMj2DkG2wNjzYLL/V+v9GuR98SzjM= X-Google-Smtp-Source: AA6agR5PwMxoxC/NFKKZygYApGZ6T9usYwLJEctGpwIJu+gxBhBTPKtGYHnjaG2sY0bOWX3XT/WMOg== X-Received: by 2002:a67:b842:0:b0:357:de51:6013 with SMTP id o2-20020a67b842000000b00357de516013mr7965929vsh.79.1660652491551; Tue, 16 Aug 2022 05:21:31 -0700 (PDT) Received: from [192.168.0.13] ([191.97.187.183]) by smtp.gmail.com with ESMTPSA id c1-20020a0561023c8100b003885fa6a5c4sm6715137vsv.9.2022.08.16.05.21.30 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 16 Aug 2022 05:21:31 -0700 (PDT) Message-ID: Date: Tue, 16 Aug 2022 09:21:31 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.12.0 Content-Language: en-US To: ffmpeg-devel@ffmpeg.org References: <20220813153601.30310-1-anton@khirnov.net> From: James Almer In-Reply-To: <20220813153601.30310-1-anton@khirnov.net> Subject: Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: store a separate copy of input codec parameters X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" Archived-At: List-Archive: List-Post: On 8/13/2022 12:36 PM, Anton Khirnov wrote: > Use it instead of AVStream.codecpar in the main thread. While > AVStream.codecpar is documented to only be updated when the stream is > added or avformat_find_stream_info(), it is actually updated during > demuxing. Accessing it from a different thread then constitutes a race. Should we consider a bug that some demuxers update stream's codepars post init? Or is the documentation not reflecting the actual behavior what's wrong? > > Ideally, some mechanism should eventually be provided for signalling > parameter updates to the user. Then the demuxing thread could pick up > the changes and propagate them to the decoder. > --- > fftools/ffmpeg.c | 39 ++++++++++++++++++++------------------- > fftools/ffmpeg.h | 6 ++++++ > fftools/ffmpeg_opt.c | 6 +++++- > 3 files changed, 31 insertions(+), 20 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 8eb7759392..ef7177fc33 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -608,6 +608,7 @@ static void ffmpeg_cleanup(int ret) > av_freep(&ist->dts_buffer); > > avcodec_free_context(&ist->dec_ctx); > + avcodec_parameters_free(&ist->par); > > av_freep(&input_streams[i]); > } > @@ -1492,7 +1493,7 @@ static void print_final_stats(int64_t total_size) > > for (j = 0; j < f->nb_streams; j++) { > InputStream *ist = input_streams[f->ist_index + j]; > - enum AVMediaType type = ist->st->codecpar->codec_type; > + enum AVMediaType type = ist->par->codec_type; > > total_size += ist->data_size; > total_packets += ist->nb_packets; > @@ -1809,7 +1810,7 @@ static void flush_encoders(void) > for (x = 0; x < fg->nb_inputs; x++) { > InputFilter *ifilter = fg->inputs[x]; > if (ifilter->format < 0 && > - ifilter_parameters_from_codecpar(ifilter, ifilter->ist->st->codecpar) < 0) { > + ifilter_parameters_from_codecpar(ifilter, ifilter->ist->par) < 0) { > av_log(NULL, AV_LOG_ERROR, "Error copying paramerets from input stream\n"); > exit_program(1); > } > @@ -1912,11 +1913,11 @@ static void do_streamcopy(InputStream *ist, OutputStream *ost, const AVPacket *p > if (pkt->dts == AV_NOPTS_VALUE) { > opkt->dts = av_rescale_q(ist->dts, AV_TIME_BASE_Q, ost->mux_timebase); > } else if (ost->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > - int duration = av_get_audio_frame_duration2(ist->st->codecpar, pkt->size); > + int duration = av_get_audio_frame_duration2(ist->par, pkt->size); > if(!duration) > - duration = ist->st->codecpar->frame_size; > + duration = ist->par->frame_size; > opkt->dts = av_rescale_delta(ist->st->time_base, pkt->dts, > - (AVRational){1, ist->st->codecpar->sample_rate}, duration, > + (AVRational){1, ist->par->sample_rate}, duration, > &ist->filter_in_rescale_delta_last, ost->mux_timebase); > /* dts will be set immediately afterwards to what pts is now */ > opkt->pts = opkt->dts - ost_tb_start_time; > @@ -1976,7 +1977,7 @@ static int ifilter_send_frame(InputFilter *ifilter, AVFrame *frame, int keep_ref > /* determine if the parameters for this input changed */ > need_reinit = ifilter->format != frame->format; > > - switch (ifilter->ist->st->codecpar->codec_type) { > + switch (ifilter->ist->par->codec_type) { > case AVMEDIA_TYPE_AUDIO: > need_reinit |= ifilter->sample_rate != frame->sample_rate || > av_channel_layout_compare(&ifilter->ch_layout, &frame->ch_layout); > @@ -2056,7 +2057,7 @@ static int ifilter_send_eof(InputFilter *ifilter, int64_t pts) > } else { > // the filtergraph was never configured > if (ifilter->format < 0) { > - ret = ifilter_parameters_from_codecpar(ifilter, ifilter->ist->st->codecpar); > + ret = ifilter_parameters_from_codecpar(ifilter, ifilter->ist->par); > if (ret < 0) > return ret; > } > @@ -2212,9 +2213,9 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_ > > // The following line may be required in some cases where there is no parser > // or the parser does not has_b_frames correctly > - if (ist->st->codecpar->video_delay < ist->dec_ctx->has_b_frames) { > + if (ist->par->video_delay < ist->dec_ctx->has_b_frames) { > if (ist->dec_ctx->codec_id == AV_CODEC_ID_H264) { > - ist->st->codecpar->video_delay = ist->dec_ctx->has_b_frames; > + ist->par->video_delay = ist->dec_ctx->has_b_frames; > } else > av_log(ist->dec_ctx, AV_LOG_WARNING, > "video_delay is larger in decoder than demuxer %d > %d.\n" > @@ -2222,7 +2223,7 @@ static int decode_video(InputStream *ist, AVPacket *pkt, int *got_output, int64_ > "of this file to https://streams.videolan.org/upload/ " > "and contact the ffmpeg-devel mailing list. (ffmpeg-devel@ffmpeg.org)\n", > ist->dec_ctx->has_b_frames, > - ist->st->codecpar->video_delay); > + ist->par->video_delay); > } > > if (ret != AVERROR_EOF) > @@ -2391,7 +2392,7 @@ static int send_filter_eof(InputStream *ist) > /* pkt = NULL means EOF (needed to flush decoder buffers) */ > static int process_input_packet(InputStream *ist, const AVPacket *pkt, int no_eof) > { > - const AVCodecParameters *par = ist->st->codecpar; > + const AVCodecParameters *par = ist->par; > int ret = 0, i; > int repeating = 0; > int eof_reached = 0; > @@ -2751,7 +2752,7 @@ static int init_output_stream_streamcopy(OutputStream *ost) > if (!codec_ctx) > return AVERROR(ENOMEM); > > - ret = avcodec_parameters_to_context(codec_ctx, ist->st->codecpar); > + ret = avcodec_parameters_to_context(codec_ctx, ist->par); > if (ret >= 0) > ret = av_opt_set_dict(codec_ctx, &ost->encoder_opts); > if (ret < 0) { > @@ -3120,8 +3121,8 @@ static int init_output_stream_encode(OutputStream *ost, AVFrame *frame) > case AVMEDIA_TYPE_SUBTITLE: > enc_ctx->time_base = AV_TIME_BASE_Q; > if (!enc_ctx->width) { > - enc_ctx->width = input_streams[ost->source_index]->st->codecpar->width; > - enc_ctx->height = input_streams[ost->source_index]->st->codecpar->height; > + enc_ctx->width = input_streams[ost->source_index]->par->width; > + enc_ctx->height = input_streams[ost->source_index]->par->height; > } > break; > case AVMEDIA_TYPE_DATA: > @@ -3678,7 +3679,7 @@ static void decode_flush(InputFile *ifile) > > if (ist->decoding_needed) { > /* report last frame duration to the demuxer thread */ > - if (ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) { > + if (ist->par->codec_type == AVMEDIA_TYPE_AUDIO) { > LastFrameDuration dur; > > dur.stream_idx = i; > @@ -3721,7 +3722,7 @@ static void ts_discontinuity_detect(InputFile *ifile, InputStream *ist, > "timestamp discontinuity for stream #%d:%d " > "(id=%d, type=%s): %"PRId64", new offset= %"PRId64"\n", > ist->file_index, ist->st->index, ist->st->id, > - av_get_media_type_string(ist->st->codecpar->codec_type), > + av_get_media_type_string(ist->par->codec_type), > delta, ifile->ts_offset_discont); > pkt->dts -= av_rescale_q(delta, AV_TIME_BASE_Q, ist->st->time_base); > if (pkt->pts != AV_NOPTS_VALUE) > @@ -3772,8 +3773,8 @@ static void ts_discontinuity_process(InputFile *ifile, InputStream *ist, > pkt->pts += offset; > > // detect timestamp discontinuities for audio/video > - if ((ist->st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO || > - ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO) && > + if ((ist->par->codec_type == AVMEDIA_TYPE_VIDEO || > + ist->par->codec_type == AVMEDIA_TYPE_AUDIO) && > pkt->dts != AV_NOPTS_VALUE) > ts_discontinuity_detect(ifile, ist, pkt); > } > @@ -3872,7 +3873,7 @@ static int process_input(int file_index) > if (debug_ts) { > av_log(NULL, AV_LOG_INFO, "demuxer+ffmpeg -> ist_index:%d type:%s pkt_pts:%s pkt_pts_time:%s pkt_dts:%s pkt_dts_time:%s duration:%s duration_time:%s off:%s off_time:%s\n", > ifile->ist_index + pkt->stream_index, > - av_get_media_type_string(ist->st->codecpar->codec_type), > + av_get_media_type_string(ist->par->codec_type), > av_ts2str(pkt->pts), av_ts2timestr(pkt->pts, &ist->st->time_base), > av_ts2str(pkt->dts), av_ts2timestr(pkt->dts, &ist->st->time_base), > av_ts2str(pkt->duration), av_ts2timestr(pkt->duration, &ist->st->time_base), > diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h > index 6991ba7632..44cc23fa84 100644 > --- a/fftools/ffmpeg.h > +++ b/fftools/ffmpeg.h > @@ -323,6 +323,12 @@ typedef struct InputStream { > #define DECODING_FOR_FILTER 2 > int processing_needed; /* non zero if the packets must be processed */ > > + /** > + * Codec parameters - to be used by the decoding/streamcopy code. > + * st->codecpar should not be accessed, because it may be modified > + * concurrently by the demuxing thread. > + */ > + AVCodecParameters *par; > AVCodecContext *dec_ctx; > const AVCodec *dec; > AVFrame *decoded_frame; > diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c > index 97f14b2a5b..30ca5cd609 100644 > --- a/fftools/ffmpeg_opt.c > +++ b/fftools/ffmpeg_opt.c > @@ -1090,7 +1090,11 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) > abort(); > } > > - ret = avcodec_parameters_from_context(par, ist->dec_ctx); > + ist->par = avcodec_parameters_alloc(); > + if (!ist->par) > + exit_program(1); > + > + ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx); > if (ret < 0) { > av_log(NULL, AV_LOG_ERROR, "Error initializing the decoder context.\n"); > exit_program(1); _______________________________________________ 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".