[FFmpeg-devel] [PATCH v2] fftools/ffmpeg: accelerate seeking while reading input at native frame rate
Gyan Doshi
ffmpeg at gyani.pro
Mon Jul 19 10:18:33 EEST 2021
On 2021-07-18 22:23, Linjie Fu wrote:
> 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
Yeah, huge difference. Will adjust commit msg and push, since seeking is
not what's affected here.
Regards,
Gyan
More information about the ffmpeg-devel
mailing list