Anton Khirnov:
> It is currently called from two places:
> - output_packet() in ffmpeg.c, which submits the newly available output
> packet to the muxer
> - from of_check_init() in ffmpeg_mux.c after the header has been
> written, to flush the muxing queue
>
> Some packets will thus be processed by this function twice, so it
> requires an extra parameter to indicate the place it is called from and
> avoid modifying some state twice.
>
> This is fragile and hard to follow, so split this function into two.
> Also rename of_write_packet() to of_submit_packet() to better reflect
> its new purpose.
> ---
> fftools/ffmpeg.c | 4 +--
> fftools/ffmpeg.h | 3 +--
> fftools/ffmpeg_mux.c | 63 ++++++++++++++++++++++++--------------------
> 3 files changed, 37 insertions(+), 33 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index e9a5c0f523..bbedf867b4 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -728,11 +728,11 @@ static void output_packet(OutputFile *of, AVPacket *pkt,
> if (ret < 0)
> goto finish;
> while ((ret = av_bsf_receive_packet(ost->bsf_ctx, pkt)) >= 0)
> - of_write_packet(of, pkt, ost, 0);
> + of_submit_packet(of, pkt, ost);
> if (ret == AVERROR(EAGAIN))
> ret = 0;
> } else if (!eof)
> - of_write_packet(of, pkt, ost, 0);
> + of_submit_packet(of, pkt, ost);
>
> finish:
> if (ret < 0 && ret != AVERROR_EOF) {
> diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
> index e6e472f994..76c8dfa4c8 100644
> --- a/fftools/ffmpeg.h
> +++ b/fftools/ffmpeg.h
> @@ -684,8 +684,7 @@ int of_check_init(OutputFile *of);
> int of_write_trailer(OutputFile *of);
> void of_close(OutputFile **pof);
>
> -void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
> - int unqueue);
> +void of_submit_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost);
> int of_finished(OutputFile *of);
> int64_t of_bytes_written(OutputFile *of);
> AVChapter * const *
> diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c
> index d4b674c9e2..e97ec8ab93 100644
> --- a/fftools/ffmpeg_mux.c
> +++ b/fftools/ffmpeg_mux.c
> @@ -102,39 +102,12 @@ static int queue_packet(OutputFile *of, OutputStream
> *ost, AVPacket *pkt)
> return 0;
> }
>
> -void of_write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost,
> - int unqueue)
> +static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt)
> {
> AVFormatContext *s = of->ctx;
> AVStream *st = ost->st;
> int ret;
>
> - /*
> - * Audio encoders may split the packets -- #frames in != #packets out.
> - * But there is no reordering, so we can limit the number of output
> packets
> - * by simply dropping them here.
> - * Counting encoded video frames needs to be done separately because of
> - * reordering, see do_video_out().
> - * Do not count the packet when unqueued because it has been counted
> when queued.
> - */
> - if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> ost->encoding_needed) && !unqueue) {
> - if (ost->frame_number >= ost->max_frames) {
> - av_packet_unref(pkt);
> - return;
> - }
> - ost->frame_number++;
> - }
Factoring this chunk out of write_packet() (effectively inlining
unqueue) looks good (and I actually pondered it myself),
> -
> - /* the muxer is not initialized yet, buffer the packet */
> - if (!of->mux->header_written) {
> - ret = queue_packet(of, ost, pkt);
> - if (ret < 0) {
> - av_packet_unref(pkt);
> - exit_program(1);
> - }
> - return;
> - }
> -
but I could not prove that the header has already been written in case
unqueue == 0. Can you guarantee this to be so and explain it to me?
> if ((st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && video_sync_method
> == VSYNC_DROP) ||
> (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && audio_sync_method
> < 0))
> pkt->pts = pkt->dts = AV_NOPTS_VALUE;
> @@ -225,6 +198,38 @@ void of_write_packet(OutputFile *of, AVPacket *pkt,
> OutputStream *ost,
> }
> }
>
> +void of_submit_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost)
> +{
> + AVStream *st = ost->st;
> + int ret;
> +
> + /*
> + * Audio encoders may split the packets -- #frames in != #packets out.
> + * But there is no reordering, so we can limit the number of output
> packets
> + * by simply dropping them here.
> + * Counting encoded video frames needs to be done separately because of
> + * reordering, see do_video_out().
> + */
> + if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> ost->encoding_needed)) {
> + if (ost->frame_number >= ost->max_frames) {
> + av_packet_unref(pkt);
> + return;
> + }
> + ost->frame_number++;
> + }
> +
> + if (of->mux->header_written) {
> + write_packet(of, ost, pkt);
> + } else {
> + /* the muxer is not initialized yet, buffer the packet */
> + ret = queue_packet(of, ost, pkt);
> + if (ret < 0) {
> + av_packet_unref(pkt);
> + exit_program(1);
> + }
> + }
> +}
> +
> static int print_sdp(void)
> {
> char sdp[16384];
> @@ -324,7 +329,7 @@ int of_check_init(OutputFile *of)
> AVPacket *pkt;
> av_fifo_generic_read(ms->muxing_queue, &pkt, sizeof(pkt), NULL);
> ms->muxing_queue_data_size -= pkt->size;
> - of_write_packet(of, pkt, ost, 1);
> + write_packet(of, ost, pkt);
> av_packet_free(&pkt);
> }
> }
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
