[FFmpeg-devel] [PATCH v2] fftools/ffmpeg: accelerate seeking while reading input at native frame rate

[Linjie Fu]

On Sun, Jul 18, 2021 at 8:57 PM Gyan Doshi <ffmpeg at gyani.pro> wrote:
>
>
>
> On 2021-07-18 15:37, Linjie Fu wrote:
> > On Sun, Jul 18, 2021 at 5:26 PM Gyan Doshi <ffmpeg at gyani.pro> wrote:
> >>
> >>
> >> On 2021-07-18 13:35, Linjie Fu wrote:
> >>> On Sun, Jul 18, 2021 at 1:21 PM Gyan Doshi <ffmpeg at 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 at 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 at gmail.com> wrote:
> >>>>>>>> On Sun, Jul 4, 2021 at 10:50 PM Linjie Fu <fulinjie at zju.edu.cn>
wrote:
> >>>>>>>>> From: Linjie Fu <linjie.justin.fu at 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 at 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