| Message ID | 20210704145023.3141-1-fulinjie@zju.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2] fftools/ffmpeg: accelerate seeking while reading input at native frame rate | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > > From: Linjie Fu <linjie.justin.fu@gmail.com> > > Skip the logic of frame rate emulation until the input reaches the > specified start time. > > Test CMD: > $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > > Before the patch: > first time to got frame, it takes 257305 us > After this patch: > first time to got frame, it takes 48879 us > > Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > --- > [v2]: fixed the mixed declaration and code warning > Calculate the time to get the first frame: > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > fftools/ffmpeg.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index e97d879cb3..c8849e4250 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > { > if (f->rate_emu) { > int i; > + int64_t pts; > + int64_t now; > for (i = 0; i < f->nb_streams; i++) { > InputStream *ist = input_streams[f->ist_index + i]; > - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > - int64_t now = av_gettime_relative() - ist->start; > + if (!ist->got_output) > + continue; > + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > + now = av_gettime_relative() - ist->start; > if (pts > now) > return AVERROR(EAGAIN); > } > -- > 2.31.1 ping, thx. - linjie
On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: > > On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > > > > From: Linjie Fu <linjie.justin.fu@gmail.com> > > > > Skip the logic of frame rate emulation until the input reaches the > > specified start time. > > > > Test CMD: > > $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > > > > Before the patch: > > first time to got frame, it takes 257305 us > > After this patch: > > first time to got frame, it takes 48879 us > > > > Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > > --- > > [v2]: fixed the mixed declaration and code warning > > Calculate the time to get the first frame: > > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > > fftools/ffmpeg.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index e97d879cb3..c8849e4250 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > > { > > if (f->rate_emu) { > > int i; > > + int64_t pts; > > + int64_t now; > > for (i = 0; i < f->nb_streams; i++) { > > InputStream *ist = input_streams[f->ist_index + i]; > > - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > > - int64_t now = av_gettime_relative() - ist->start; > > + if (!ist->got_output) > > + continue; > > + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > > + now = av_gettime_relative() - ist->start; > > if (pts > now) > > return AVERROR(EAGAIN); > > } > > -- > > 2.31.1 > ping, thx. > Another ping, thx.
On 2021-07-18 09:32, Linjie Fu wrote: > On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: >> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: >>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>> >>> Skip the logic of frame rate emulation until the input reaches the >>> specified start time. >>> >>> Test CMD: >>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>> >>> Before the patch: >>> first time to got frame, it takes 257305 us >>> After this patch: >>> first time to got frame, it takes 48879 us >>> >>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>> --- >>> [v2]: fixed the mixed declaration and code warning >>> Calculate the time to get the first frame: >>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>> fftools/ffmpeg.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>> index e97d879cb3..c8849e4250 100644 >>> --- a/fftools/ffmpeg.c >>> +++ b/fftools/ffmpeg.c >>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) >>> { >>> if (f->rate_emu) { >>> int i; >>> + int64_t pts; >>> + int64_t now; >>> for (i = 0; i < f->nb_streams; i++) { >>> InputStream *ist = input_streams[f->ist_index + i]; >>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>> - int64_t now = av_gettime_relative() - ist->start; >>> + if (!ist->got_output) >>> + continue; >>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>> + now = av_gettime_relative() - ist->start; >>> if (pts > now) >>> return AVERROR(EAGAIN); >>> } >>> -- >>> 2.31.1 >> ping, thx. >> > Another ping, thx. I pushed changes to this code yesterday. I don't think it's required anymore, but do test. Regards, Gyan
Hi Gyan, On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2021-07-18 09:32, Linjie Fu wrote: > > On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: > >> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > >>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>> > >>> Skip the logic of frame rate emulation until the input reaches the > >>> specified start time. > >>> > >>> Test CMD: > >>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > >>> > >>> Before the patch: > >>> first time to got frame, it takes 257305 us > >>> After this patch: > >>> first time to got frame, it takes 48879 us > >>> > >>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > >>> --- > >>> [v2]: fixed the mixed declaration and code warning > >>> Calculate the time to get the first frame: > >>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >>> fftools/ffmpeg.c | 8 ++++++-- > >>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>> index e97d879cb3..c8849e4250 100644 > >>> --- a/fftools/ffmpeg.c > >>> +++ b/fftools/ffmpeg.c > >>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > >>> { > >>> if (f->rate_emu) { > >>> int i; > >>> + int64_t pts; > >>> + int64_t now; > >>> for (i = 0; i < f->nb_streams; i++) { > >>> InputStream *ist = input_streams[f->ist_index + i]; > >>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>> - int64_t now = av_gettime_relative() - ist->start; > >>> + if (!ist->got_output) > >>> + continue; > >>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>> + now = av_gettime_relative() - ist->start; > >>> if (pts > now) > >>> return AVERROR(EAGAIN); > >>> } > >>> -- > >>> 2.31.1 > >> ping, thx. > >> > > Another ping, thx. > > I pushed changes to this code yesterday. I don't think it's required > anymore, but do test. > Thanks for the review, tested after applying the readrate patch, I'm afraid that it's not identical as hope, since ist->nb_packets would increase no matter input stream got output or not: (lldb) p ist->nb_packets (uint64_t) $4 = 1 (lldb) p ist->got_output (int) $5 = 0 Hence we still need to add the check for ist->got_output, or replace the ist->nb_packets. Also there is a new warning caught by the check in patchwork, probably "mixed declaration and code warning". Will send patches to rebase and fix. - linjie
On 2021-07-18 10:42, Linjie Fu wrote: > Hi Gyan, > On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> On 2021-07-18 09:32, Linjie Fu wrote: >>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: >>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: >>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>>>> >>>>> Skip the logic of frame rate emulation until the input reaches the >>>>> specified start time. >>>>> >>>>> Test CMD: >>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>>>> >>>>> Before the patch: >>>>> first time to got frame, it takes 257305 us >>>>> After this patch: >>>>> first time to got frame, it takes 48879 us >>>>> >>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>>>> --- >>>>> [v2]: fixed the mixed declaration and code warning >>>>> Calculate the time to get the first frame: >>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>>>> fftools/ffmpeg.c | 8 ++++++-- >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>> index e97d879cb3..c8849e4250 100644 >>>>> --- a/fftools/ffmpeg.c >>>>> +++ b/fftools/ffmpeg.c >>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) >>>>> { >>>>> if (f->rate_emu) { >>>>> int i; >>>>> + int64_t pts; >>>>> + int64_t now; >>>>> for (i = 0; i < f->nb_streams; i++) { >>>>> InputStream *ist = input_streams[f->ist_index + i]; >>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>> - int64_t now = av_gettime_relative() - ist->start; >>>>> + if (!ist->got_output) >>>>> + continue; >>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>> + now = av_gettime_relative() - ist->start; >>>>> if (pts > now) >>>>> return AVERROR(EAGAIN); >>>>> } >>>>> -- >>>>> 2.31.1 >>>> ping, thx. >>>> >>> Another ping, thx. >> I pushed changes to this code yesterday. I don't think it's required >> anymore, but do test. >> > Thanks for the review, tested after applying the readrate patch, I'm > afraid that it's not identical as hope, > since ist->nb_packets would increase no matter input stream got output or not: > > (lldb) p ist->nb_packets > (uint64_t) $4 = 1 > > (lldb) p ist->got_output > (int) $5 = 0 > > Hence we still need to add the check for ist->got_output, or replace > the ist->nb_packets. No, test the speed, not the parity of got_output. got_output is only incremented when the stream is decoded. Won't work with streamcopy. nb_packets is the correct check since it's incremented after the initial packet is demuxed. > Also there is a new warning caught by the check in patchwork, probably > "mixed declaration and code warning". > Will send patches to rebase and fix. There is already patch for the mixed warning. Regards, Gyan
On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2021-07-18 10:42, Linjie Fu wrote: > > Hi Gyan, > > On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >> > >> > >> On 2021-07-18 09:32, Linjie Fu wrote: > >>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: > >>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > >>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>> > >>>>> Skip the logic of frame rate emulation until the input reaches the > >>>>> specified start time. > >>>>> > >>>>> Test CMD: > >>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > >>>>> > >>>>> Before the patch: > >>>>> first time to got frame, it takes 257305 us > >>>>> After this patch: > >>>>> first time to got frame, it takes 48879 us > >>>>> > >>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>> --- > >>>>> [v2]: fixed the mixed declaration and code warning > >>>>> Calculate the time to get the first frame: > >>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >>>>> fftools/ffmpeg.c | 8 ++++++-- > >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>>> > >>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>>>> index e97d879cb3..c8849e4250 100644 > >>>>> --- a/fftools/ffmpeg.c > >>>>> +++ b/fftools/ffmpeg.c > >>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > >>>>> { > >>>>> if (f->rate_emu) { > >>>>> int i; > >>>>> + int64_t pts; > >>>>> + int64_t now; > >>>>> for (i = 0; i < f->nb_streams; i++) { > >>>>> InputStream *ist = input_streams[f->ist_index + i]; > >>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>> - int64_t now = av_gettime_relative() - ist->start; > >>>>> + if (!ist->got_output) > >>>>> + continue; > >>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>> + now = av_gettime_relative() - ist->start; > >>>>> if (pts > now) > >>>>> return AVERROR(EAGAIN); > >>>>> } > >>>>> -- > >>>>> 2.31.1 > >>>> ping, thx. > >>>> > >>> Another ping, thx. > >> I pushed changes to this code yesterday. I don't think it's required > >> anymore, but do test. > >> > > Thanks for the review, tested after applying the readrate patch, I'm > > afraid that it's not identical as hope, > > since ist->nb_packets would increase no matter input stream got output or not: > > > > (lldb) p ist->nb_packets > > (uint64_t) $4 = 1 > > > > (lldb) p ist->got_output > > (int) $5 = 0 > > > > Hence we still need to add the check for ist->got_output, or replace > > the ist->nb_packets. > > No, test the speed, not the parity of got_output. got_output is only > incremented when the stream is decoded. The time interval consumed to get the first frame is tested as well, it's not speeded up: First time to get the frame, it takes 257395 us. After adding the check for got_output in decoding scenery, this would reduce to 48792 us. Hence it doesn't reduce the latency under decode scenery. > Won't work with streamcopy. Got your point. > nb_packets is the correct check since it's incremented after the initial > packet is demuxed. IIRC, the initial packet is demuxed right after the first time we call get_input_packet()-> av_read_frame(), and nb_packets would increase. Hence, the check for ist->nb_packets would reduce the latency for demuxing the first packet, and won't help much if we use start_time like "-ss 30". Under the decoding scenery, we still need to wait until the pkt->pts specified time then demux the packet. If no objections, consider to change the check to: if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) continue; to reduce the waiting time for: 1. demuxing the first packet (if any) 2. demuxing the packet before we got the first decoded output. > > Also there is a new warning caught by the check in patchwork, probably > > "mixed declaration and code warning". > > Will send patches to rebase and fix. > > There is already patch for the mixed warning. Saw it, thx. - linjie
On 2021-07-18 13:35, Linjie Fu wrote: > On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> On 2021-07-18 10:42, Linjie Fu wrote: >>> Hi Gyan, >>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>> >>>> On 2021-07-18 09:32, Linjie Fu wrote: >>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: >>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: >>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>> >>>>>>> Skip the logic of frame rate emulation until the input reaches the >>>>>>> specified start time. >>>>>>> >>>>>>> Test CMD: >>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>>>>>> >>>>>>> Before the patch: >>>>>>> first time to got frame, it takes 257305 us >>>>>>> After this patch: >>>>>>> first time to got frame, it takes 48879 us >>>>>>> >>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>> --- >>>>>>> [v2]: fixed the mixed declaration and code warning >>>>>>> Calculate the time to get the first frame: >>>>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>>>>>> fftools/ffmpeg.c | 8 ++++++-- >>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>>>> index e97d879cb3..c8849e4250 100644 >>>>>>> --- a/fftools/ffmpeg.c >>>>>>> +++ b/fftools/ffmpeg.c >>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) >>>>>>> { >>>>>>> if (f->rate_emu) { >>>>>>> int i; >>>>>>> + int64_t pts; >>>>>>> + int64_t now; >>>>>>> for (i = 0; i < f->nb_streams; i++) { >>>>>>> InputStream *ist = input_streams[f->ist_index + i]; >>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>>>> - int64_t now = av_gettime_relative() - ist->start; >>>>>>> + if (!ist->got_output) >>>>>>> + continue; >>>>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>>>> + now = av_gettime_relative() - ist->start; >>>>>>> if (pts > now) >>>>>>> return AVERROR(EAGAIN); >>>>>>> } >>>>>>> -- >>>>>>> 2.31.1 >>>>>> ping, thx. >>>>>> >>>>> Another ping, thx. >>>> I pushed changes to this code yesterday. I don't think it's required >>>> anymore, but do test. >>>> >>> Thanks for the review, tested after applying the readrate patch, I'm >>> afraid that it's not identical as hope, >>> since ist->nb_packets would increase no matter input stream got output or not: >>> >>> (lldb) p ist->nb_packets >>> (uint64_t) $4 = 1 >>> >>> (lldb) p ist->got_output >>> (int) $5 = 0 >>> >>> Hence we still need to add the check for ist->got_output, or replace >>> the ist->nb_packets. >> No, test the speed, not the parity of got_output. got_output is only >> incremented when the stream is decoded. > The time interval consumed to get the first frame is tested as well, > it's not speeded up: > First time to get the frame, it takes 257395 us. > After adding the check for got_output in decoding scenery, this would > reduce to 48792 us. > > Hence it doesn't reduce the latency under decode scenery. > >> Won't work with streamcopy. > Got your point. > >> nb_packets is the correct check since it's incremented after the initial >> packet is demuxed. > IIRC, the initial packet is demuxed right after the first time we call > get_input_packet()-> av_read_frame(), and nb_packets would increase. > Hence, the check for ist->nb_packets would reduce the latency for > demuxing the first packet, > and won't help much if we use start_time like "-ss 30". Under the > decoding scenery, we still > need to wait until the pkt->pts specified time then demux the packet. > > If no objections, consider to change the check to: > > if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) continue; > > to reduce the waiting time for: > 1. demuxing the first packet (if any) > 2. demuxing the packet before we got the first decoded output. Which sample file are you using, and how do I reproduce the timings? Regards, Gyan
On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2021-07-18 13:35, Linjie Fu wrote: > > On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >> > >> > >> On 2021-07-18 10:42, Linjie Fu wrote: > >>> Hi Gyan, > >>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >>>> > >>>> On 2021-07-18 09:32, Linjie Fu wrote: > >>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: > >>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > >>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>> > >>>>>>> Skip the logic of frame rate emulation until the input reaches the > >>>>>>> specified start time. > >>>>>>> > >>>>>>> Test CMD: > >>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > >>>>>>> > >>>>>>> Before the patch: > >>>>>>> first time to got frame, it takes 257305 us > >>>>>>> After this patch: > >>>>>>> first time to got frame, it takes 48879 us > >>>>>>> > >>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>> --- > >>>>>>> [v2]: fixed the mixed declaration and code warning > >>>>>>> Calculate the time to get the first frame: > >>>>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >>>>>>> fftools/ffmpeg.c | 8 ++++++-- > >>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>>>>> > >>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>>>>>> index e97d879cb3..c8849e4250 100644 > >>>>>>> --- a/fftools/ffmpeg.c > >>>>>>> +++ b/fftools/ffmpeg.c > >>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > >>>>>>> { > >>>>>>> if (f->rate_emu) { > >>>>>>> int i; > >>>>>>> + int64_t pts; > >>>>>>> + int64_t now; > >>>>>>> for (i = 0; i < f->nb_streams; i++) { > >>>>>>> InputStream *ist = input_streams[f->ist_index + i]; > >>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>>>> - int64_t now = av_gettime_relative() - ist->start; > >>>>>>> + if (!ist->got_output) > >>>>>>> + continue; > >>>>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>>>> + now = av_gettime_relative() - ist->start; > >>>>>>> if (pts > now) > >>>>>>> return AVERROR(EAGAIN); > >>>>>>> } > >>>>>>> -- > >>>>>>> 2.31.1 > >>>>>> ping, thx. > >>>>>> > >>>>> Another ping, thx. > >>>> I pushed changes to this code yesterday. I don't think it's required > >>>> anymore, but do test. > >>>> > >>> Thanks for the review, tested after applying the readrate patch, I'm > >>> afraid that it's not identical as hope, > >>> since ist->nb_packets would increase no matter input stream got output or not: > >>> > >>> (lldb) p ist->nb_packets > >>> (uint64_t) $4 = 1 > >>> > >>> (lldb) p ist->got_output > >>> (int) $5 = 0 > >>> > >>> Hence we still need to add the check for ist->got_output, or replace > >>> the ist->nb_packets. > >> No, test the speed, not the parity of got_output. got_output is only > >> incremented when the stream is decoded. > > The time interval consumed to get the first frame is tested as well, > > it's not speeded up: > > First time to get the frame, it takes 257395 us. > > After adding the check for got_output in decoding scenery, this would > > reduce to 48792 us. > > > > Hence it doesn't reduce the latency under decode scenery. > > > >> Won't work with streamcopy. > > Got your point. > > > >> nb_packets is the correct check since it's incremented after the initial > >> packet is demuxed. > > IIRC, the initial packet is demuxed right after the first time we call > > get_input_packet()-> av_read_frame(), and nb_packets would increase. > > Hence, the check for ist->nb_packets would reduce the latency for > > demuxing the first packet, > > and won't help much if we use start_time like "-ss 30". Under the > > decoding scenery, we still > > need to wait until the pkt->pts specified time then demux the packet. > > > > If no objections, consider to change the check to: > > > > if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) continue; > > > > to reduce the waiting time for: > > 1. demuxing the first packet (if any) > > 2. demuxing the packet before we got the first decoded output. > > Which sample file are you using, and how do I reproduce the timings? > It could be reproduced with a random input file (if long enough), and the time calculating method I used previously (needs to rebase a little bit for now): https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed CMD: $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - or $ffmpeg -re -ss 30 -i input.mp4 -f null - - linjie
On 2021-07-18 15:37, Linjie Fu wrote: > On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> On 2021-07-18 13:35, Linjie Fu wrote: >>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>> >>>> On 2021-07-18 10:42, Linjie Fu wrote: >>>>> Hi Gyan, >>>>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>>> On 2021-07-18 09:32, Linjie Fu wrote: >>>>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu <linjie.justin.fu@gmail.com> wrote: >>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: >>>>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>> >>>>>>>>> Skip the logic of frame rate emulation until the input reaches the >>>>>>>>> specified start time. >>>>>>>>> >>>>>>>>> Test CMD: >>>>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>>>>>>>> >>>>>>>>> Before the patch: >>>>>>>>> first time to got frame, it takes 257305 us >>>>>>>>> After this patch: >>>>>>>>> first time to got frame, it takes 48879 us >>>>>>>>> >>>>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>> --- >>>>>>>>> [v2]: fixed the mixed declaration and code warning >>>>>>>>> Calculate the time to get the first frame: >>>>>>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>>>>>>>> fftools/ffmpeg.c | 8 ++++++-- >>>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>>>>>> index e97d879cb3..c8849e4250 100644 >>>>>>>>> --- a/fftools/ffmpeg.c >>>>>>>>> +++ b/fftools/ffmpeg.c >>>>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) >>>>>>>>> { >>>>>>>>> if (f->rate_emu) { >>>>>>>>> int i; >>>>>>>>> + int64_t pts; >>>>>>>>> + int64_t now; >>>>>>>>> for (i = 0; i < f->nb_streams; i++) { >>>>>>>>> InputStream *ist = input_streams[f->ist_index + i]; >>>>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>>>>>> - int64_t now = av_gettime_relative() - ist->start; >>>>>>>>> + if (!ist->got_output) >>>>>>>>> + continue; >>>>>>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>>>>>> + now = av_gettime_relative() - ist->start; >>>>>>>>> if (pts > now) >>>>>>>>> return AVERROR(EAGAIN); >>>>>>>>> } >>>>>>>>> -- >>>>>>>>> 2.31.1 >>>>>>>> ping, thx. >>>>>>>> >>>>>>> Another ping, thx. >>>>>> I pushed changes to this code yesterday. I don't think it's required >>>>>> anymore, but do test. >>>>>> >>>>> Thanks for the review, tested after applying the readrate patch, I'm >>>>> afraid that it's not identical as hope, >>>>> since ist->nb_packets would increase no matter input stream got output or not: >>>>> >>>>> (lldb) p ist->nb_packets >>>>> (uint64_t) $4 = 1 >>>>> >>>>> (lldb) p ist->got_output >>>>> (int) $5 = 0 >>>>> >>>>> Hence we still need to add the check for ist->got_output, or replace >>>>> the ist->nb_packets. >>>> No, test the speed, not the parity of got_output. got_output is only >>>> incremented when the stream is decoded. >>> The time interval consumed to get the first frame is tested as well, >>> it's not speeded up: >>> First time to get the frame, it takes 257395 us. >>> After adding the check for got_output in decoding scenery, this would >>> reduce to 48792 us. >>> >>> Hence it doesn't reduce the latency under decode scenery. >>> >>>> Won't work with streamcopy. >>> Got your point. >>> >>>> nb_packets is the correct check since it's incremented after the initial >>>> packet is demuxed. >>> IIRC, the initial packet is demuxed right after the first time we call >>> get_input_packet()-> av_read_frame(), and nb_packets would increase. >>> Hence, the check for ist->nb_packets would reduce the latency for >>> demuxing the first packet, >>> and won't help much if we use start_time like "-ss 30". Under the >>> decoding scenery, we still >>> need to wait until the pkt->pts specified time then demux the packet. >>> >>> If no objections, consider to change the check to: >>> >>> if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) continue; >>> >>> to reduce the waiting time for: >>> 1. demuxing the first packet (if any) >>> 2. demuxing the packet before we got the first decoded output. >> Which sample file are you using, and how do I reproduce the timings? >> > It could be reproduced with a random input file (if long enough), and > the time calculating method I used previously (needs to rebase a > little bit for now): > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > > CMD: > $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > or > $ffmpeg -re -ss 30 -i input.mp4 -f null - I don't see a difference. See attached text file. Regards, Gyan Input file downloaded from https://download.blender.org/durian/trailer/sintel_trailer-720p.mp4 ---PATCHED-1----- static int get_input_packet(InputFile *f, AVPacket **pkt) { if (f->readrate || f->rate_emu) { int i; int64_t file_start = copy_ts * ( (f->ctx->start_time != AV_NOPTS_VALUE ? f->ctx->start_time * !start_at_zero : 0) + (f->start_time != AV_NOPTS_VALUE ? f->start_time : 0) ); float scale = f->rate_emu ? 1.0 : f->readrate; for (i = 0; i < f->nb_streams; i++) { InputStream *ist = input_streams[f->ist_index + i]; int64_t stream_ts_offset = FFMAX(ist->first_dts != AV_NOPTS_VALUE ? ist->first_dts : 0, file_start); int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); int64_t now = (av_gettime_relative() - ist->start)*scale + stream_ts_offset; av_log(NULL, AV_LOG_WARNING, "#%d:%d |nb_packets: %"PRId64" | pts: %"PRId64" |now: %"PRId64" |decoded: %d |got_output: %d\n", ist->file_index, i, ist->nb_packets, pts, now, ist->decoding_needed, ist->got_output); if (!ist->nb_packets) continue; if (pts > now) return AVERROR(EAGAIN); } } -------- ---PATCHED-2----- static int get_input_packet(InputFile *f, AVPacket **pkt) { if (f->readrate || f->rate_emu) { int i; int64_t file_start = copy_ts * ( (f->ctx->start_time != AV_NOPTS_VALUE ? f->ctx->start_time * !start_at_zero : 0) + (f->start_time != AV_NOPTS_VALUE ? f->start_time : 0) ); float scale = f->rate_emu ? 1.0 : f->readrate; for (i = 0; i < f->nb_streams; i++) { InputStream *ist = input_streams[f->ist_index + i]; int64_t stream_ts_offset = FFMAX(ist->first_dts != AV_NOPTS_VALUE ? ist->first_dts : 0, file_start); int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); int64_t now = (av_gettime_relative() - ist->start)*scale + stream_ts_offset; av_log(NULL, AV_LOG_WARNING, "#%d:%d |nb_packets: %"PRId64" | pts: %"PRId64" |now: %"PRId64" |decoded: %d |got_output: %d\n", ist->file_index, i, ist->nb_packets, pts, now, ist->decoding_needed, ist->got_output); if (!ist->nb_packets || (ist->decoding_needed && !ist->got_output)) continue; if (pts > now) return AVERROR(EAGAIN); } } -------- (Difference is between line 19 and line 42) Decode command: ffmpeg -re -ss 30 -i sintel_trailer-720p.mp4 -an -vframes 24 -f null - -v 24 Streamcopy command: ffmpeg -re -ss 30 -i sintel_trailer-720p.mp4 -c copy -an -vframes 24 -f null - -v 24 Results ======= (shown till first frame output) PATCHED-1 Decoded #0:0 |nb_packets: 0 | pts: 0 |now: 776 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 904 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 1 | pts: -2666667 |now: 1062 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1178 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 2 | pts: -2625000 |now: 1779 |decoded: 1 |got_output: 0 <-- time to demux second packet = 1779 us #0:1 |nb_packets: 0 | pts: 0 |now: 1898 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 3 | pts: -2583333 |now: 2460 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2597 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 4 | pts: -2541667 |now: 3184 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3318 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 5 | pts: -2500000 |now: 4045 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 4174 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 6 | pts: -2458333 |now: 4715 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 4873 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 7 | pts: -2416667 |now: 5386 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 5527 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 8 | pts: -2375000 |now: 6032 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 6164 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 9 | pts: -2333333 |now: 6701 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 6832 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 10 | pts: -2291667 |now: 7367 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 7722 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 11 | pts: -2250000 |now: 8411 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 8599 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 12 | pts: -2208333 |now: 9331 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 9507 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 13 | pts: -2166667 |now: 10186 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 10461 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 14 | pts: -2125000 |now: 10757 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 10894 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 15 | pts: -2041666 |now: 12543 |decoded: 1 |got_output: 1 <-- time to obtain first frame, after 15 pkts = 12543 us PATCHED-2 Decoded #0:0 |nb_packets: 0 | pts: 0 |now: 780 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 841 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 1 | pts: -2666667 |now: 932 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 992 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 2 | pts: -2625000 |now: 1645 |decoded: 1 |got_output: 0 <-- time to demux second packet = 1645 us #0:1 |nb_packets: 0 | pts: 0 |now: 1747 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 3 | pts: -2583333 |now: 2311 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2371 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 4 | pts: -2541667 |now: 3058 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3123 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 5 | pts: -2500000 |now: 3864 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3929 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 6 | pts: -2458333 |now: 4558 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 4624 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 7 | pts: -2416667 |now: 5218 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 5281 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 8 | pts: -2375000 |now: 5886 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 6023 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 9 | pts: -2333333 |now: 6675 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 6801 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 10 | pts: -2291667 |now: 7774 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 7926 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 11 | pts: -2250000 |now: 8887 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 9026 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 12 | pts: -2208333 |now: 9624 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 9752 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 13 | pts: -2166667 |now: 10277 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 10423 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 14 | pts: -2125000 |now: 10847 |decoded: 1 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 10991 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 15 | pts: -2041666 |now: 12619 |decoded: 1 |got_output: 1 <-- time to obtain first frame, after 15 pkts = 12619 us PATCHED-1 Streamcopied #0:0 |nb_packets: 0 | pts: 0 |now: 77 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 198 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 1 | pts: -2666667 |now: 346 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 463 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 2 | pts: -2625000 |now: 585 |decoded: 0 |got_output: 0 <-- time to demux second packet = 585 us #0:1 |nb_packets: 0 | pts: 0 |now: 701 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 3 | pts: -2583333 |now: 819 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 936 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 4 | pts: -2541667 |now: 1064 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1181 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 5 | pts: -2500000 |now: 1295 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1410 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 6 | pts: -2458333 |now: 1528 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1644 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 7 | pts: -2416667 |now: 1760 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1875 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 8 | pts: -2375000 |now: 1994 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2109 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 9 | pts: -2333333 |now: 2235 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2351 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 10 | pts: -2291667 |now: 2467 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2582 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 11 | pts: -2250000 |now: 2698 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2813 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 12 | pts: -2208333 |now: 2928 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3046 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 13 | pts: -2166667 |now: 3163 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3278 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 14 | pts: -2125000 |now: 3396 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3510 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 15 | pts: -2083333 |now: 3626 |decoded: 0 |got_output: 0 <-- time after 15 pkts = 3626 us PATCHED-2 Streamcopied #0:0 |nb_packets: 0 | pts: 0 |now: 74 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 191 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 1 | pts: -2666667 |now: 337 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 452 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 2 | pts: -2625000 |now: 575 |decoded: 0 |got_output: 0 <-- time to demux second packet = 575 us #0:1 |nb_packets: 0 | pts: 0 |now: 691 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 3 | pts: -2583333 |now: 808 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 922 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 4 | pts: -2541667 |now: 1045 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1160 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 5 | pts: -2500000 |now: 1309 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1423 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 6 | pts: -2458333 |now: 1654 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 1865 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 7 | pts: -2416667 |now: 1987 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2103 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 8 | pts: -2375000 |now: 2219 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2334 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 9 | pts: -2333333 |now: 2458 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2574 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 10 | pts: -2291667 |now: 2691 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 2805 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 11 | pts: -2250000 |now: 2920 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3033 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 12 | pts: -2208333 |now: 3148 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3262 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 13 | pts: -2166667 |now: 3376 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3489 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 14 | pts: -2125000 |now: 3605 |decoded: 0 |got_output: 0 #0:1 |nb_packets: 0 | pts: 0 |now: 3718 |decoded: 0 |got_output: 0 #0:0 |nb_packets: 15 | pts: -2083333 |now: 3834 |decoded: 0 |got_output: 0 <-- time after 15 pkts = 3834 us
On Sun, Jul 18, 2021 at 8:57 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2021-07-18 15:37, Linjie Fu wrote: > > On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >> > >> > >> On 2021-07-18 13:35, Linjie Fu wrote: > >>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >>>> > >>>> On 2021-07-18 10:42, Linjie Fu wrote: > >>>>> Hi Gyan, > >>>>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >>>>>> On 2021-07-18 09:32, Linjie Fu wrote: > >>>>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu < linjie.justin.fu@gmail.com> wrote: > >>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> wrote: > >>>>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>>>> > >>>>>>>>> Skip the logic of frame rate emulation until the input reaches the > >>>>>>>>> specified start time. > >>>>>>>>> > >>>>>>>>> Test CMD: > >>>>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > >>>>>>>>> > >>>>>>>>> Before the patch: > >>>>>>>>> first time to got frame, it takes 257305 us > >>>>>>>>> After this patch: > >>>>>>>>> first time to got frame, it takes 48879 us > >>>>>>>>> > >>>>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>>>> --- > >>>>>>>>> [v2]: fixed the mixed declaration and code warning > >>>>>>>>> Calculate the time to get the first frame: > >>>>>>>>> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >>>>>>>>> fftools/ffmpeg.c | 8 ++++++-- > >>>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>>>>>>> > >>>>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>>>>>>>> index e97d879cb3..c8849e4250 100644 > >>>>>>>>> --- a/fftools/ffmpeg.c > >>>>>>>>> +++ b/fftools/ffmpeg.c > >>>>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) > >>>>>>>>> { > >>>>>>>>> if (f->rate_emu) { > >>>>>>>>> int i; > >>>>>>>>> + int64_t pts; > >>>>>>>>> + int64_t now; > >>>>>>>>> for (i = 0; i < f->nb_streams; i++) { > >>>>>>>>> InputStream *ist = input_streams[f->ist_index + i]; > >>>>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>>>>>> - int64_t now = av_gettime_relative() - ist->start; > >>>>>>>>> + if (!ist->got_output) > >>>>>>>>> + continue; > >>>>>>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); > >>>>>>>>> + now = av_gettime_relative() - ist->start; > >>>>>>>>> if (pts > now) > >>>>>>>>> return AVERROR(EAGAIN); > >>>>>>>>> } > >>>>>>>>> -- > >>>>>>>>> 2.31.1 > >>>>>>>> ping, thx. > >>>>>>>> > >>>>>>> Another ping, thx. > >>>>>> I pushed changes to this code yesterday. I don't think it's required > >>>>>> anymore, but do test. > >>>>>> > >>>>> Thanks for the review, tested after applying the readrate patch, I'm > >>>>> afraid that it's not identical as hope, > >>>>> since ist->nb_packets would increase no matter input stream got output or not: > >>>>> > >>>>> (lldb) p ist->nb_packets > >>>>> (uint64_t) $4 = 1 > >>>>> > >>>>> (lldb) p ist->got_output > >>>>> (int) $5 = 0 > >>>>> > >>>>> Hence we still need to add the check for ist->got_output, or replace > >>>>> the ist->nb_packets. > >>>> No, test the speed, not the parity of got_output. got_output is only > >>>> incremented when the stream is decoded. > >>> The time interval consumed to get the first frame is tested as well, > >>> it's not speeded up: > >>> First time to get the frame, it takes 257395 us. > >>> After adding the check for got_output in decoding scenery, this would > >>> reduce to 48792 us. > >>> > >>> Hence it doesn't reduce the latency under decode scenery. > >>> > >>>> Won't work with streamcopy. > >>> Got your point. > >>> > >>>> nb_packets is the correct check since it's incremented after the initial > >>>> packet is demuxed. > >>> IIRC, the initial packet is demuxed right after the first time we call > >>> get_input_packet()-> av_read_frame(), and nb_packets would increase. > >>> Hence, the check for ist->nb_packets would reduce the latency for > >>> demuxing the first packet, > >>> and won't help much if we use start_time like "-ss 30". Under the > >>> decoding scenery, we still > >>> need to wait until the pkt->pts specified time then demux the packet. > >>> > >>> If no objections, consider to change the check to: > >>> > >>> if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) continue; > >>> > >>> to reduce the waiting time for: > >>> 1. demuxing the first packet (if any) > >>> 2. demuxing the packet before we got the first decoded output. > >> Which sample file are you using, and how do I reproduce the timings? > >> > > It could be reproduced with a random input file (if long enough), and > > the time calculating method I used previously (needs to rebase a > > little bit for now): > > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > > > > CMD: > > $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > > or > > $ffmpeg -re -ss 30 -i input.mp4 -f null - > > I don't see a difference. See attached text file. > Checked the attached logs with decoding : ffmpeg -re -ss 30 -i sintel_trailer-720p.mp4 -an -vframes 24 -f null - -v 24 #0:0 |nb_packets: 14 | pts: -2125000 |now: 10757 |decoded: 1 |got_output: 0 #0:0 |nb_packets: 15 | pts: -2041666 |now: 12543 |decoded: 1 |got_output: 1 <-- time to obtain first frame, after 15 pkts = 12543 us In this case the "pts" is always less than "now", hence there is no wait and won't enter into the logic of "EAGAIN return". So no difference is observed. Here is another example in which "pts" may be larger than "now" before we got the output: https://docs.google.com/document/d/19fTcGMvTC6surQTvJPNNnngUrTEIRnqElu2lCCU6qwU/edit?usp=sharing - linjie
On 2021-07-18 22:23, Linjie Fu wrote: > On Sun, Jul 18, 2021 at 8:57 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> On 2021-07-18 15:37, Linjie Fu wrote: >>> On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>> >>>> On 2021-07-18 13:35, Linjie Fu wrote: >>>>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>>> On 2021-07-18 10:42, Linjie Fu wrote: >>>>>>> Hi Gyan, >>>>>>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> > wrote: >>>>>>>> On 2021-07-18 09:32, Linjie Fu wrote: >>>>>>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu < > linjie.justin.fu@gmail.com> wrote: >>>>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> > wrote: >>>>>>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>>>> >>>>>>>>>>> Skip the logic of frame rate emulation until the input reaches > the >>>>>>>>>>> specified start time. >>>>>>>>>>> >>>>>>>>>>> Test CMD: >>>>>>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 > - >>>>>>>>>>> Before the patch: >>>>>>>>>>> first time to got frame, it takes 257305 us >>>>>>>>>>> After this patch: >>>>>>>>>>> first time to got frame, it takes 48879 us >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>>>> --- >>>>>>>>>>> [v2]: fixed the mixed declaration and code warning >>>>>>>>>>> Calculate the time to get the first frame: >>>>>>>>>>> > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>>>>>>>>>> fftools/ffmpeg.c | 8 ++++++-- >>>>>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>>>>>>>> index e97d879cb3..c8849e4250 100644 >>>>>>>>>>> --- a/fftools/ffmpeg.c >>>>>>>>>>> +++ b/fftools/ffmpeg.c >>>>>>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile > *f, AVPacket **pkt) >>>>>>>>>>> { >>>>>>>>>>> if (f->rate_emu) { >>>>>>>>>>> int i; >>>>>>>>>>> + int64_t pts; >>>>>>>>>>> + int64_t now; >>>>>>>>>>> for (i = 0; i < f->nb_streams; i++) { >>>>>>>>>>> InputStream *ist = input_streams[f->ist_index > + i]; >>>>>>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, > AV_TIME_BASE); >>>>>>>>>>> - int64_t now = av_gettime_relative() - ist->start; >>>>>>>>>>> + if (!ist->got_output) >>>>>>>>>>> + continue; >>>>>>>>>>> + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); >>>>>>>>>>> + now = av_gettime_relative() - ist->start; >>>>>>>>>>> if (pts > now) >>>>>>>>>>> return AVERROR(EAGAIN); >>>>>>>>>>> } >>>>>>>>>>> -- >>>>>>>>>>> 2.31.1 >>>>>>>>>> ping, thx. >>>>>>>>>> >>>>>>>>> Another ping, thx. >>>>>>>> I pushed changes to this code yesterday. I don't think it's > required >>>>>>>> anymore, but do test. >>>>>>>> >>>>>>> Thanks for the review, tested after applying the readrate patch, I'm >>>>>>> afraid that it's not identical as hope, >>>>>>> since ist->nb_packets would increase no matter input stream got > output or not: >>>>>>> (lldb) p ist->nb_packets >>>>>>> (uint64_t) $4 = 1 >>>>>>> >>>>>>> (lldb) p ist->got_output >>>>>>> (int) $5 = 0 >>>>>>> >>>>>>> Hence we still need to add the check for ist->got_output, or > replace >>>>>>> the ist->nb_packets. >>>>>> No, test the speed, not the parity of got_output. got_output is only >>>>>> incremented when the stream is decoded. >>>>> The time interval consumed to get the first frame is tested as well, >>>>> it's not speeded up: >>>>> First time to get the frame, it takes 257395 us. >>>>> After adding the check for got_output in decoding scenery, this would >>>>> reduce to 48792 us. >>>>> >>>>> Hence it doesn't reduce the latency under decode scenery. >>>>> >>>>>> Won't work with streamcopy. >>>>> Got your point. >>>>> >>>>>> nb_packets is the correct check since it's incremented after the > initial >>>>>> packet is demuxed. >>>>> IIRC, the initial packet is demuxed right after the first time we call >>>>> get_input_packet()-> av_read_frame(), and nb_packets would increase. >>>>> Hence, the check for ist->nb_packets would reduce the latency for >>>>> demuxing the first packet, >>>>> and won't help much if we use start_time like "-ss 30". Under the >>>>> decoding scenery, we still >>>>> need to wait until the pkt->pts specified time then demux the packet. >>>>> >>>>> If no objections, consider to change the check to: >>>>> >>>>> if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) > continue; >>>>> to reduce the waiting time for: >>>>> 1. demuxing the first packet (if any) >>>>> 2. demuxing the packet before we got the first decoded output. >>>> Which sample file are you using, and how do I reproduce the timings? >>>> >>> It could be reproduced with a random input file (if long enough), and >>> the time calculating method I used previously (needs to rebase a >>> little bit for now): >>> > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >>> CMD: >>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>> or >>> $ffmpeg -re -ss 30 -i input.mp4 -f null - >> I don't see a difference. See attached text file. >> > Checked the attached logs with decoding : ffmpeg -re -ss 30 -i > sintel_trailer-720p.mp4 -an -vframes 24 -f null - -v 24 > > #0:0 |nb_packets: 14 | pts: -2125000 |now: 10757 |decoded: 1 |got_output: 0 > #0:0 |nb_packets: 15 | pts: -2041666 |now: 12543 |decoded: 1 |got_output: 1 > <-- time to obtain first frame, after 15 pkts = 12543 us > > In this case the "pts" is always less than "now", hence there is no wait > and won't enter into the logic of "EAGAIN return". > So no difference is observed. > > Here is another example in which "pts" may be larger than "now" before we > got the output: > https://docs.google.com/document/d/19fTcGMvTC6surQTvJPNNnngUrTEIRnqElu2lCCU6qwU/edit?usp=sharing Yeah, huge difference. Will adjust commit msg and push, since seeking is not what's affected here. Regards, Gyan
On 2021-07-19 12:48, Gyan Doshi wrote: > > > On 2021-07-18 22:23, Linjie Fu wrote: >> On Sun, Jul 18, 2021 at 8:57 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>> >>> >>> On 2021-07-18 15:37, Linjie Fu wrote: >>>> On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>> >>>>> On 2021-07-18 13:35, Linjie Fu wrote: >>>>>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>>>> On 2021-07-18 10:42, Linjie Fu wrote: >>>>>>>> Hi Gyan, >>>>>>>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> >> wrote: >>>>>>>>> On 2021-07-18 09:32, Linjie Fu wrote: >>>>>>>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu < >> linjie.justin.fu@gmail.com> wrote: >>>>>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn> >> wrote: >>>>>>>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>>>>> >>>>>>>>>>>> Skip the logic of frame rate emulation until the input reaches >> the >>>>>>>>>>>> specified start time. >>>>>>>>>>>> >>>>>>>>>>>> Test CMD: >>>>>>>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f >>>>>>>>>>>> sdl2 >> - >>>>>>>>>>>> Before the patch: >>>>>>>>>>>> first time to got frame, it takes 257305 us >>>>>>>>>>>> After this patch: >>>>>>>>>>>> first time to got frame, it takes 48879 us >>>>>>>>>>>> >>>>>>>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> >>>>>>>>>>>> --- >>>>>>>>>>>> [v2]: fixed the mixed declaration and code warning >>>>>>>>>>>> Calculate the time to get the first frame: >>>>>>>>>>>> >> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >> >>>>>>>>>>>> fftools/ffmpeg.c | 8 ++++++-- >>>>>>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >>>>>>>>>>>> index e97d879cb3..c8849e4250 100644 >>>>>>>>>>>> --- a/fftools/ffmpeg.c >>>>>>>>>>>> +++ b/fftools/ffmpeg.c >>>>>>>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile >> *f, AVPacket **pkt) >>>>>>>>>>>> { >>>>>>>>>>>> if (f->rate_emu) { >>>>>>>>>>>> int i; >>>>>>>>>>>> + int64_t pts; >>>>>>>>>>>> + int64_t now; >>>>>>>>>>>> for (i = 0; i < f->nb_streams; i++) { >>>>>>>>>>>> InputStream *ist = >>>>>>>>>>>> input_streams[f->ist_index >> + i]; >>>>>>>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, >> AV_TIME_BASE); >>>>>>>>>>>> - int64_t now = av_gettime_relative() - ist->start; >>>>>>>>>>>> + if (!ist->got_output) >>>>>>>>>>>> + continue; >>>>>>>>>>>> + pts = av_rescale(ist->dts, 1000000, >>>>>>>>>>>> AV_TIME_BASE); >>>>>>>>>>>> + now = av_gettime_relative() - ist->start; >>>>>>>>>>>> if (pts > now) >>>>>>>>>>>> return AVERROR(EAGAIN); >>>>>>>>>>>> } >>>>>>>>>>>> -- >>>>>>>>>>>> 2.31.1 >>>>>>>>>>> ping, thx. >>>>>>>>>>> >>>>>>>>>> Another ping, thx. >>>>>>>>> I pushed changes to this code yesterday. I don't think it's >> required >>>>>>>>> anymore, but do test. >>>>>>>>> >>>>>>>> Thanks for the review, tested after applying the readrate >>>>>>>> patch, I'm >>>>>>>> afraid that it's not identical as hope, >>>>>>>> since ist->nb_packets would increase no matter input stream got >> output or not: >>>>>>>> (lldb) p ist->nb_packets >>>>>>>> (uint64_t) $4 = 1 >>>>>>>> >>>>>>>> (lldb) p ist->got_output >>>>>>>> (int) $5 = 0 >>>>>>>> >>>>>>>> Hence we still need to add the check for ist->got_output, or >> replace >>>>>>>> the ist->nb_packets. >>>>>>> No, test the speed, not the parity of got_output. got_output is >>>>>>> only >>>>>>> incremented when the stream is decoded. >>>>>> The time interval consumed to get the first frame is tested as well, >>>>>> it's not speeded up: >>>>>> First time to get the frame, it takes 257395 us. >>>>>> After adding the check for got_output in decoding scenery, this >>>>>> would >>>>>> reduce to 48792 us. >>>>>> >>>>>> Hence it doesn't reduce the latency under decode scenery. >>>>>> >>>>>>> Won't work with streamcopy. >>>>>> Got your point. >>>>>> >>>>>>> nb_packets is the correct check since it's incremented after the >> initial >>>>>>> packet is demuxed. >>>>>> IIRC, the initial packet is demuxed right after the first time we >>>>>> call >>>>>> get_input_packet()-> av_read_frame(), and nb_packets would >>>>>> increase. >>>>>> Hence, the check for ist->nb_packets would reduce the latency for >>>>>> demuxing the first packet, >>>>>> and won't help much if we use start_time like "-ss 30". Under the >>>>>> decoding scenery, we still >>>>>> need to wait until the pkt->pts specified time then demux the >>>>>> packet. >>>>>> >>>>>> If no objections, consider to change the check to: >>>>>> >>>>>> if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) >> continue; >>>>>> to reduce the waiting time for: >>>>>> 1. demuxing the first packet (if any) >>>>>> 2. demuxing the packet before we got the first decoded output. >>>>> Which sample file are you using, and how do I reproduce the timings? >>>>> >>>> It could be reproduced with a random input file (if long enough), and >>>> the time calculating method I used previously (needs to rebase a >>>> little bit for now): >>>> >> https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed >> >>>> CMD: >>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - >>>> or >>>> $ffmpeg -re -ss 30 -i input.mp4 -f null - >>> I don't see a difference. See attached text file. >>> >> Checked the attached logs with decoding : ffmpeg -re -ss 30 -i >> sintel_trailer-720p.mp4 -an -vframes 24 -f null - -v 24 >> >> #0:0 |nb_packets: 14 | pts: -2125000 |now: 10757 |decoded: 1 >> |got_output: 0 >> #0:0 |nb_packets: 15 | pts: -2041666 |now: 12543 |decoded: 1 >> |got_output: 1 >> <-- time to obtain first frame, after 15 pkts = 12543 us >> >> In this case the "pts" is always less than "now", hence there is no wait >> and won't enter into the logic of "EAGAIN return". >> So no difference is observed. >> >> Here is another example in which "pts" may be larger than "now" >> before we >> got the output: >> https://docs.google.com/document/d/19fTcGMvTC6surQTvJPNNnngUrTEIRnqElu2lCCU6qwU/edit?usp=sharing >> > > Yeah, huge difference. Will adjust commit msg and push, since seeking > is not what's affected here. New patch pushed as 6f206852289ee8997bef6a43a88252834d2d3e02 Thanks, Gyan
On Mon, Jul 19, 2021 at 3:32 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > On 2021-07-19 12:48, Gyan Doshi wrote: > > > > > > On 2021-07-18 22:23, Linjie Fu wrote: > >> On Sun, Jul 18, 2021 at 8:57 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >>> > >>> > >>> On 2021-07-18 15:37, Linjie Fu wrote: > >>>> On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg@gyani.pro> wrote: > >>>>> > >>>>> On 2021-07-18 13:35, Linjie Fu wrote: > >>>>>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg@gyani.pro> > wrote: > >>>>>>> On 2021-07-18 10:42, Linjie Fu wrote: > >>>>>>>> Hi Gyan, > >>>>>>>> On Sun, Jul 18, 2021 at 12:24 PM Gyan Doshi <ffmpeg@gyani.pro> > >> wrote: > >>>>>>>>> On 2021-07-18 09:32, Linjie Fu wrote: > >>>>>>>>>> On Wed, Jul 7, 2021 at 9:42 AM Linjie Fu < > >> linjie.justin.fu@gmail.com> wrote: > >>>>>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie@zju.edu.cn > > > >> wrote: > >>>>>>>>>>>> From: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>>>>>>> > >>>>>>>>>>>> Skip the logic of frame rate emulation until the input reaches > >> the > >>>>>>>>>>>> specified start time. > >>>>>>>>>>>> > >>>>>>>>>>>> Test CMD: > >>>>>>>>>>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f > >>>>>>>>>>>> sdl2 > >> - > >>>>>>>>>>>> Before the patch: > >>>>>>>>>>>> first time to got frame, it takes 257305 us > >>>>>>>>>>>> After this patch: > >>>>>>>>>>>> first time to got frame, it takes 48879 us > >>>>>>>>>>>> > >>>>>>>>>>>> Signed-off-by: Linjie Fu <linjie.justin.fu@gmail.com> > >>>>>>>>>>>> --- > >>>>>>>>>>>> [v2]: fixed the mixed declaration and code warning > >>>>>>>>>>>> Calculate the time to get the first frame: > >>>>>>>>>>>> > >> > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >> > >>>>>>>>>>>> fftools/ffmpeg.c | 8 ++++++-- > >>>>>>>>>>>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>>>>>>>>>>> > >>>>>>>>>>>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>>>>>>>>>>> index e97d879cb3..c8849e4250 100644 > >>>>>>>>>>>> --- a/fftools/ffmpeg.c > >>>>>>>>>>>> +++ b/fftools/ffmpeg.c > >>>>>>>>>>>> @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile > >> *f, AVPacket **pkt) > >>>>>>>>>>>> { > >>>>>>>>>>>> if (f->rate_emu) { > >>>>>>>>>>>> int i; > >>>>>>>>>>>> + int64_t pts; > >>>>>>>>>>>> + int64_t now; > >>>>>>>>>>>> for (i = 0; i < f->nb_streams; i++) { > >>>>>>>>>>>> InputStream *ist = > >>>>>>>>>>>> input_streams[f->ist_index > >> + i]; > >>>>>>>>>>>> - int64_t pts = av_rescale(ist->dts, 1000000, > >> AV_TIME_BASE); > >>>>>>>>>>>> - int64_t now = av_gettime_relative() - ist->start; > >>>>>>>>>>>> + if (!ist->got_output) > >>>>>>>>>>>> + continue; > >>>>>>>>>>>> + pts = av_rescale(ist->dts, 1000000, > >>>>>>>>>>>> AV_TIME_BASE); > >>>>>>>>>>>> + now = av_gettime_relative() - ist->start; > >>>>>>>>>>>> if (pts > now) > >>>>>>>>>>>> return AVERROR(EAGAIN); > >>>>>>>>>>>> } > >>>>>>>>>>>> -- > >>>>>>>>>>>> 2.31.1 > >>>>>>>>>>> ping, thx. > >>>>>>>>>>> > >>>>>>>>>> Another ping, thx. > >>>>>>>>> I pushed changes to this code yesterday. I don't think it's > >> required > >>>>>>>>> anymore, but do test. > >>>>>>>>> > >>>>>>>> Thanks for the review, tested after applying the readrate > >>>>>>>> patch, I'm > >>>>>>>> afraid that it's not identical as hope, > >>>>>>>> since ist->nb_packets would increase no matter input stream got > >> output or not: > >>>>>>>> (lldb) p ist->nb_packets > >>>>>>>> (uint64_t) $4 = 1 > >>>>>>>> > >>>>>>>> (lldb) p ist->got_output > >>>>>>>> (int) $5 = 0 > >>>>>>>> > >>>>>>>> Hence we still need to add the check for ist->got_output, or > >> replace > >>>>>>>> the ist->nb_packets. > >>>>>>> No, test the speed, not the parity of got_output. got_output is > >>>>>>> only > >>>>>>> incremented when the stream is decoded. > >>>>>> The time interval consumed to get the first frame is tested as well, > >>>>>> it's not speeded up: > >>>>>> First time to get the frame, it takes 257395 us. > >>>>>> After adding the check for got_output in decoding scenery, this > >>>>>> would > >>>>>> reduce to 48792 us. > >>>>>> > >>>>>> Hence it doesn't reduce the latency under decode scenery. > >>>>>> > >>>>>>> Won't work with streamcopy. > >>>>>> Got your point. > >>>>>> > >>>>>>> nb_packets is the correct check since it's incremented after the > >> initial > >>>>>>> packet is demuxed. > >>>>>> IIRC, the initial packet is demuxed right after the first time we > >>>>>> call > >>>>>> get_input_packet()-> av_read_frame(), and nb_packets would > >>>>>> increase. > >>>>>> Hence, the check for ist->nb_packets would reduce the latency for > >>>>>> demuxing the first packet, > >>>>>> and won't help much if we use start_time like "-ss 30". Under the > >>>>>> decoding scenery, we still > >>>>>> need to wait until the pkt->pts specified time then demux the > >>>>>> packet. > >>>>>> > >>>>>> If no objections, consider to change the check to: > >>>>>> > >>>>>> if (!ist->nb_packets || (!ist->got_output && ist->decoding_needed)) > >> continue; > >>>>>> to reduce the waiting time for: > >>>>>> 1. demuxing the first packet (if any) > >>>>>> 2. demuxing the packet before we got the first decoded output. > >>>>> Which sample file are you using, and how do I reproduce the timings? > >>>>> > >>>> It could be reproduced with a random input file (if long enough), and > >>>> the time calculating method I used previously (needs to rebase a > >>>> little bit for now): > >>>> > >> > https://github.com/fulinjie/ffmpeg/commit/2aa4762e1e65709997b1ab9dd596332244db80ed > >> > >>>> CMD: > >>>> $ffmpeg -re -ss 30 -i input.mp4 -pix_fmt yuv420p -f sdl2 - > >>>> or > >>>> $ffmpeg -re -ss 30 -i input.mp4 -f null - > >>> I don't see a difference. See attached text file. > >>> > >> Checked the attached logs with decoding : ffmpeg -re -ss 30 -i > >> sintel_trailer-720p.mp4 -an -vframes 24 -f null - -v 24 > >> > >> #0:0 |nb_packets: 14 | pts: -2125000 |now: 10757 |decoded: 1 > >> |got_output: 0 > >> #0:0 |nb_packets: 15 | pts: -2041666 |now: 12543 |decoded: 1 > >> |got_output: 1 > >> <-- time to obtain first frame, after 15 pkts = 12543 us > >> > >> In this case the "pts" is always less than "now", hence there is no wait > >> and won't enter into the logic of "EAGAIN return". > >> So no difference is observed. > >> > >> Here is another example in which "pts" may be larger than "now" > >> before we > >> got the output: > >> > https://docs.google.com/document/d/19fTcGMvTC6surQTvJPNNnngUrTEIRnqElu2lCCU6qwU/edit?usp=sharing > >> > > > > Yeah, huge difference. Will adjust commit msg and push, since seeking > > is not what's affected here. > > New patch pushed as 6f206852289ee8997bef6a43a88252834d2d3e02 > Thanks for the effort. - linjie
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index e97d879cb3..c8849e4250 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -4221,10 +4221,14 @@ static int get_input_packet(InputFile *f, AVPacket **pkt) { if (f->rate_emu) { int i; + int64_t pts; + int64_t now; for (i = 0; i < f->nb_streams; i++) { InputStream *ist = input_streams[f->ist_index + i]; - int64_t pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); - int64_t now = av_gettime_relative() - ist->start; + if (!ist->got_output) + continue; + pts = av_rescale(ist->dts, 1000000, AV_TIME_BASE); + now = av_gettime_relative() - ist->start; if (pts > now) return AVERROR(EAGAIN); }