diff mbox series

[FFmpeg-devel,RFC,GSoC,v1,6/6] ffplay: add av_packet_buffer_filter to filter packet buffer

Message ID 20200715083733.101880-6-sj.hc_Zhong@sjtu.edu.cn
State Superseded
Headers show
Series [FFmpeg-devel,RFC,GSoC,v1,1/6] avformat/abr: Adaptive Bitrate support
Related show

Checks

Context Check Description
andriy/default pending
andriy/make fail Make failed

Commit Message

Hongcheng Zhong July 15, 2020, 8:37 a.m. UTC
From: spartazhc <spartazhc@gmail.com>

hls_read_header will add all streams to s->internal->packet_buffer.
Use av_packet_buffer_filter to remove the AVPackets from other streams that
are not needed, otherwise abr will allow them to be added to ffplay's
packet_queue.

Signed-off-by: spartazhc <spartazhc@gmail.com>
---
 fftools/ffplay.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Andreas Rheinhardt July 16, 2020, 4:11 p.m. UTC | #1
Hongcheng Zhong:
> From: spartazhc <spartazhc@gmail.com>
> 
> hls_read_header will add all streams to s->internal->packet_buffer.
> Use av_packet_buffer_filter to remove the AVPackets from other streams that
> are not needed, otherwise abr will allow them to be added to ffplay's
> packet_queue.
> 
> Signed-off-by: spartazhc <spartazhc@gmail.com>
> ---
>  fftools/ffplay.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> index b17b75fa8f..832e97d910 100644
> --- a/fftools/ffplay.c
> +++ b/fftools/ffplay.c
> @@ -3018,6 +3018,10 @@ static int read_thread(void *arg)
>  
>      /* clean packet list filled in hls_read_header if abr is enabled */
>      if (abr) {
> +        ret = av_packet_buffer_filter(ic, st_index);
> +        if (ret < 0) {
> +            av_log(NULL, AV_LOG_WARNING, "Failed to clean av_packet\n");
> +        }
>          is->abr_list = av_mallocz(sizeof(ABRList));
>          ret = abr_init_list(is);
>          if (ret < 0) {
> 
Do we need this new function at all? Can't we not just set
AVStream.discard to AVDISCARD_ALL for the streams we don't want and
modify av_read_frame() to make sure that it never outputs packets
belonging to a stream that ought to be discarded?

- Andreas
Hongcheng Zhong July 19, 2020, 11:41 a.m. UTC | #2
On Thu, 2020-07-16 at 18:11 +0200, Andreas Rheinhardt wrote:
> Hongcheng Zhong:
> > From: spartazhc <spartazhc@gmail.com>
> > 
> > hls_read_header will add all streams to s->internal->packet_buffer.
> > Use av_packet_buffer_filter to remove the AVPackets from other
> > streams that
> > are not needed, otherwise abr will allow them to be added to
> > ffplay's
> > packet_queue.
> > 
> > Signed-off-by: spartazhc <spartazhc@gmail.com>
> > ---
> >  fftools/ffplay.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fftools/ffplay.c b/fftools/ffplay.c
> > index b17b75fa8f..832e97d910 100644
> > --- a/fftools/ffplay.c
> > +++ b/fftools/ffplay.c
> > @@ -3018,6 +3018,10 @@ static int read_thread(void *arg)
> >  
> >      /* clean packet list filled in hls_read_header if abr is
> > enabled */
> >      if (abr) {
> > +        ret = av_packet_buffer_filter(ic, st_index);
> > +        if (ret < 0) {
> > +            av_log(NULL, AV_LOG_WARNING, "Failed to clean
> > av_packet\n");
> > +        }
> >          is->abr_list = av_mallocz(sizeof(ABRList));
> >          ret = abr_init_list(is);
> >          if (ret < 0) {
> > 
> Do we need this new function at all? Can't we not just set
> AVStream.discard to AVDISCARD_ALL for the streams we don't want and
> modify av_read_frame() to make sure that it never outputs packets
> belonging to a stream that ought to be discarded?
> 
> - Andreas
> _______________________________________________
I am not sure if it can be done elegantly. Currently in ffplay, all
stream packets will be added to packet list in ffplay's
read_thread avformat_find_stream_info (all these are before
AVDISCARD_ALL are set to streams). After this, ffplay will call
av_find_best_stream() to decide which stream to be reserved.

I tried to setup AVDISCARD flags after hls_read_header(), which will
prevent av_read_frame() to read other streams. But there are 2 problems

1. I can use variant's bandwidth only to determine which one is the
best stream, not av_find_best_stream(). It will cause problem if two
results are different, so I even need to disable av_find_best_stream().

2. Will cause warning "Could not find codec parameters for stream 2
(Video: h264, 1 reference frame ([27][0][0][0] / 0x001B), none):
unspecified size Consider increasing the value for the analyzeduration'
(0) and 'probesize' (5000000) options".

I have considered about it when I added this API. Adding an API is an
easy way to fix the problem, but not good enough.

Really need your advice here.

Regards,
Hongcheng
diff mbox series

Patch

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index b17b75fa8f..832e97d910 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -3018,6 +3018,10 @@  static int read_thread(void *arg)
 
     /* clean packet list filled in hls_read_header if abr is enabled */
     if (abr) {
+        ret = av_packet_buffer_filter(ic, st_index);
+        if (ret < 0) {
+            av_log(NULL, AV_LOG_WARNING, "Failed to clean av_packet\n");
+        }
         is->abr_list = av_mallocz(sizeof(ABRList));
         ret = abr_init_list(is);
         if (ret < 0) {