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

---
 fftools/ffplay.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

On Sun, 19 Nov 2017, John Stebbins wrote:

> ---
> fftools/ffplay.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index 10a917194d..97555d5047 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -257,6 +257,7 @@ typedef struct VideoState {
>     struct SwrContext *swr_ctx;
>     int frame_drops_early;
>     int frame_drops_late;
> +    int drop_disposable;
>
>     enum ShowMode {
>         SHOW_MODE_NONE = -1, SHOW_MODE_VIDEO = 0, SHOW_MODE_WAVES, SHOW_MODE_RDFT, SHOW_MODE_NB
> @@ -1619,9 +1620,11 @@ retry:
>                 duration = vp_duration(is, vp, nextvp);
>                 if(!is->step && (framedrop>0 || (framedrop && get_master_sync_type(is) != AV_SYNC_VIDEO_MASTER)) && time > is->frame_timer + duration){
>                     is->frame_drops_late++;
> +                    is->drop_disposable = 1;
>                     frame_queue_next(&is->pictq);
>                     goto retry;
>                 }
> +                is->drop_disposable = 0;
>             }
>
>             if (is->subtitle_st) {
> @@ -2900,6 +2903,7 @@ static int read_thread(void *arg)
>         infinite_buffer = 1;
>
>     for (;;) {
> +        AVStream * st;
>         if (is->abort_request)
>             break;
>         if (is->paused != is->last_paused) {
> @@ -3008,6 +3012,12 @@ static int read_thread(void *arg)
>         } else {
>             is->eof = 0;
>         }
> +        st = ic->streams[pkt->stream_index];
> +        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
> +            is->drop_disposable && pkt->flags & AV_PKT_FLAG_DISPOSABLE) {
> +            av_packet_unref(pkt);
> +            continue;
> +        }
>         /* check if packet is in play range specified by user, then queue, otherwise discard */
>         stream_start_time = ic->streams[pkt->stream_index]->start_time;
>         pkt_ts = pkt->pts == AV_NOPTS_VALUE ? pkt->dts : pkt->pts;

I am not a fan of enabling this by default. Unknown number of packets 
accumulate in the packet queue, unknown number of packets are processed by 
the decoder, so we might drop packets referring to frames 1 second from 
now... If you really want to dynamically drop packets, then at least drop 
them right before feeding them to the decoder to get rid of the packet 
queue latency.

Regards,
Marton

On 11/19/2017 01:28 PM, Marton Balint wrote:
> On Sun, 19 Nov 2017, John Stebbins wrote:
>
>> ---
>> fftools/ffplay.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
>> index 10a917194d..97555d5047 100644
>> --- a/fftools/ffplay.c
>> +++ b/fftools/ffplay.c
>> @@ -257,6 +257,7 @@ typedef struct VideoState {
>>     struct SwrContext *swr_ctx;
>>     int frame_drops_early;
>>     int frame_drops_late;
>> +    int drop_disposable;
>>
>>     enum ShowMode {
>>         SHOW_MODE_NONE = -1, SHOW_MODE_VIDEO = 0, SHOW_MODE_WAVES, SHOW_MODE_RDFT, SHOW_MODE_NB
>> @@ -1619,9 +1620,11 @@ retry:
>>                 duration = vp_duration(is, vp, nextvp);
>>                 if(!is->step && (framedrop>0 || (framedrop && get_master_sync_type(is) != AV_SYNC_VIDEO_MASTER)) && time > is->frame_timer + duration){
>>                     is->frame_drops_late++;
>> +                    is->drop_disposable = 1;
>>                     frame_queue_next(&is->pictq);
>>                     goto retry;
>>                 }
>> +                is->drop_disposable = 0;
>>             }
>>
>>             if (is->subtitle_st) {
>> @@ -2900,6 +2903,7 @@ static int read_thread(void *arg)
>>         infinite_buffer = 1;
>>
>>     for (;;) {
>> +        AVStream * st;
>>         if (is->abort_request)
>>             break;
>>         if (is->paused != is->last_paused) {
>> @@ -3008,6 +3012,12 @@ static int read_thread(void *arg)
>>         } else {
>>             is->eof = 0;
>>         }
>> +        st = ic->streams[pkt->stream_index];
>> +        if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
>> +            is->drop_disposable && pkt->flags & AV_PKT_FLAG_DISPOSABLE) {
>> +            av_packet_unref(pkt);
>> +            continue;
>> +        }
>>         /* check if packet is in play range specified by user, then queue, otherwise discard */
>>         stream_start_time = ic->streams[pkt->stream_index]->start_time;
>>         pkt_ts = pkt->pts == AV_NOPTS_VALUE ? pkt->dts : pkt->pts;
> I am not a fan of enabling this by default. Unknown number of packets 
> accumulate in the packet queue, unknown number of packets are processed by 
> the decoder, so we might drop packets referring to frames 1 second from 
> now... If you really want to dynamically drop packets, then at least drop 
> them right before feeding them to the decoder to get rid of the packet 
> queue latency.
>
>

The way it's coded, it can be disabled with -noframedrop.  But that would disable all frame dropping.  I can add a
separate option for this if you prefer.  And I'll move it to where packets are sent to the decoder to minimize buffering
effects.  This patch was primarily created as a means of testing that the correct frames were being marked as
disposable.  My feelings wouldn't be hurt if you asked to drop this entirely ;)