[FFmpeg-devel] [PATCH 6/6] ffplay: use AV_PKT_FLAG_DISPOSABLE in frame drop logic

[Marton Balint]

On Mon, 4 Dec 2017, John Stebbins wrote:

> On 12/03/2017 01:12 PM, Marton Balint wrote:
>> On Thu, 30 Nov 2017, John Stebbins wrote:
>>
>>> ---
>>> fftools/ffplay.c | 21 ++++++++++++++++-----
>>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
>>> index 10a917194d..152d220cdb 100644
>>> --- a/fftools/ffplay.c
>>> +++ b/fftools/ffplay.c
>>> @@ -198,6 +198,8 @@ typedef struct Decoder {
>>>     int64_t next_pts;
>>>     AVRational next_pts_tb;
>>>     SDL_Thread *decoder_tid;
>>> +    int drop_disposable;
>>> +    int frame_drops_disposable;
>>> } Decoder;
>>>
>>> typedef struct VideoState {
>>> @@ -660,10 +662,16 @@ static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) {
>>>                     ret = got_frame ? 0 : (pkt.data ? AVERROR(EAGAIN) : AVERROR_EOF);
>>>                 }
>>>             } else {
>>> -                if (avcodec_send_packet(d->avctx, &pkt) == AVERROR(EAGAIN)) {
>>> -                    av_log(d->avctx, AV_LOG_ERROR, "Receive_frame and send_packet both returned EAGAIN, which is an API violation.\n");
>>> -                    d->packet_pending = 1;
>>> -                    av_packet_move_ref(&d->pkt, &pkt);
>>> +                if (d->avctx->codec_type == AVMEDIA_TYPE_VIDEO &&
>>> +                    d->drop_disposable &&
>>> +                    (pkt.flags & AV_PKT_FLAG_DISPOSABLE)) {
>>> +                    d->frame_drops_disposable++;
>>> +                } else {
>>> +                    if (avcodec_send_packet(d->avctx, &pkt) == AVERROR(EAGAIN)) {
>>> +                        av_log(d->avctx, AV_LOG_ERROR, "Receive_frame and send_packet both returned EAGAIN, which is an API violation.\n");
>>> +                        d->packet_pending = 1;
>>> +                        av_packet_move_ref(&d->pkt, &pkt);
>>> +                    }
>>>                 }
>>>             }
>>>             av_packet_unref(&pkt);
>>> @@ -1622,6 +1630,7 @@ retry:
>>>                     frame_queue_next(&is->pictq);
>>>                     goto retry;
>>>                 }
>>> +                is->viddec.drop_disposable = 0;
>>>             }
>>>
>>>             if (is->subtitle_st) {
>>> @@ -1699,7 +1708,8 @@ display:
>>>                    get_master_clock(is),
>>>                    (is->audio_st && is->video_st) ? "A-V" : (is->video_st ? "M-V" : (is->audio_st ? "M-A" : "   ")),
>>>                    av_diff,
>>> -                   is->frame_drops_early + is->frame_drops_late,
>>> +                   is->frame_drops_early + is->frame_drops_late +
>>> +                                           is->viddec.frame_drops_disposable,
>>>                    aqsize / 1024,
>>>                    vqsize / 1024,
>>>                    sqsize,
>>> @@ -1767,6 +1777,7 @@ static int get_video_frame(VideoState *is, AVFrame *frame)
>>>                     is->frame_drops_early++;
>>>                     av_frame_unref(frame);
>>>                     got_picture = 0;
>>> +                    is->viddec.drop_disposable = 1;
>>>                 }
>>>             }
>>>         }
>> The patch looks OK now, but as I mentioned earlier, please do not enable
>> this kind of frame dropping by default, either introduce a separate option
>> "hardframedrop", or make -framedrop an integer and use "2" for this kind
>> of hard dropping.
>>
>>
>
> I can certainly do that.  But I don't really understand the reason behind it.  Frame dropping is already happening in
> the scenario where this would be enabled.  There are 2 types of frame dropping that currently happen in ffplay, "early"
> and "late".  "Early" frame dropping happens when it is detected that the frame is too late for it's time slot
> immediately after it is decoded.  "Late" frame dropping happens when it is detected that the frame is too late when it
> is removed from the display queue. Dropping disposable frames in when "early" frame dropping is happening gives the
> player a better chance of actually being able to catch up to the current time slot because it bypasses decoding those
> frames.  Without it only the first few of frames get displayed with the samples I've been testing.  After those first
> few, decoding is slow enough that all subsequent frames are too late and get dropped by the "early" frame drop logic.  
> With disposable frame dropping, the video plays reasonably well (with a stutter here and there).

Ok, let's keep the patch as is then.

Regards,
Marton