[FFmpeg-devel] avformat/hls: fix compiling error

Submitted by Steven Liu on Dec. 25, 2017, 3:48 a.m.

Details

Message ID 20171225034808.29895-1-lq@chinaffmpeg.org
State New
Headers show

Commit Message

Steven Liu Dec. 25, 2017, 3:48 a.m.
fix --disable-network compipling error

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
---
 libavformat/hls.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Aman Gupta Dec. 25, 2017, 5:21 p.m.
On Sun, Dec 24, 2017 at 7:48 PM Steven Liu <lq@chinaffmpeg.org> wrote:

> fix --disable-network compipling error
>
> Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> ---
>  libavformat/hls.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index f00e22dfef..51d83b7557 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -611,14 +611,16 @@ static void update_options(char **dest, const char
> *name, void *src)
>  static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
>                                const char *url)
>  {
> -    int ret;
> +    int ret = 0;


Returning 0 does not make sense since that means no error.


>      URLContext *uc = ffio_geturlcontext(*pb);
>      av_assert0(uc);


This will trigger an assertion failure and crash.


>      (*pb)->eof_reached = 0;
> +#if CONFIG_HTTP_PROTOCOL
>      ret = ff_http_do_new_request(uc, url);
>      if (ret < 0) {
>          ff_format_io_close(s, pb);
>      }
> +#endif


>      return ret;
>  }


I think it would be better to #if the entire function body, and return some
error code in the case where the feature is not available, like
AVERROR_PROTOCOL_NOT_FOUND

Aman


>
> --
> 2.14.3 (Apple Git-98)
>
>
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
wm4 Dec. 25, 2017, 11:46 p.m.
On Mon, 25 Dec 2017 17:21:23 +0000
Aman Gupta <ffmpeg@tmm1.net> wrote:

> On Sun, Dec 24, 2017 at 7:48 PM Steven Liu <lq@chinaffmpeg.org> wrote:
> 
> > fix --disable-network compipling error
> >
> > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > ---
> >  libavformat/hls.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index f00e22dfef..51d83b7557 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -611,14 +611,16 @@ static void update_options(char **dest, const char
> > *name, void *src)
> >  static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
> >                                const char *url)
> >  {
> > -    int ret;
> > +    int ret = 0;  
> 
> 
> Returning 0 does not make sense since that means no error.
> 
> 
> >      URLContext *uc = ffio_geturlcontext(*pb);
> >      av_assert0(uc);  
> 
> 
> This will trigger an assertion failure and crash.
> 
> 
> >      (*pb)->eof_reached = 0;
> > +#if CONFIG_HTTP_PROTOCOL
> >      ret = ff_http_do_new_request(uc, url);
> >      if (ret < 0) {
> >          ff_format_io_close(s, pb);
> >      }
> > +#endif  
> 
> 
> >      return ret;
> >  }  
> 
> 
> I think it would be better to #if the entire function body, and return some
> error code in the case where the feature is not available, like
> AVERROR_PROTOCOL_NOT_FOUND

Or a callback, to make this part of native AVIO or URLContext (or
whatever it's using). That would avoid all ifdeffery and build
fragility.
Aman Gupta Dec. 26, 2017, 6:58 p.m.
On Mon, Dec 25, 2017 at 3:46 PM wm4 <nfxjfg@googlemail.com> wrote:

> On Mon, 25 Dec 2017 17:21:23 +0000
> Aman Gupta <ffmpeg@tmm1.net> wrote:
>
> > On Sun, Dec 24, 2017 at 7:48 PM Steven Liu <lq@chinaffmpeg.org> wrote:
> >
> > > fix --disable-network compipling error
> > >
> > > Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
> > > ---
> > >  libavformat/hls.c | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > > index f00e22dfef..51d83b7557 100644
> > > --- a/libavformat/hls.c
> > > +++ b/libavformat/hls.c
> > > @@ -611,14 +611,16 @@ static void update_options(char **dest, const
> char
> > > *name, void *src)
> > >  static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
> > >                                const char *url)
> > >  {
> > > -    int ret;
> > > +    int ret = 0;
> >
> >
> > Returning 0 does not make sense since that means no error.
> >
> >
> > >      URLContext *uc = ffio_geturlcontext(*pb);
> > >      av_assert0(uc);
> >
> >
> > This will trigger an assertion failure and crash.
> >
> >
> > >      (*pb)->eof_reached = 0;
> > > +#if CONFIG_HTTP_PROTOCOL
> > >      ret = ff_http_do_new_request(uc, url);
> > >      if (ret < 0) {
> > >          ff_format_io_close(s, pb);
> > >      }
> > > +#endif
> >
> >
> > >      return ret;
> > >  }
> >
> >
> > I think it would be better to #if the entire function body, and return
> some
> > error code in the case where the feature is not available, like
> > AVERROR_PROTOCOL_NOT_FOUND
>
> Or a callback, to make this part of native AVIO or URLContext (or
> whatever it's using). That would avoid all ifdeffery and build
> fragility.


I'm struggling to understand what you mean.

Currently we have an AVIOContext which wraps an URLContext, which wraps an
HTTPContext.

What callback are you proposing, and where would it go?

Aman


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

diff --git a/libavformat/hls.c b/libavformat/hls.c
index f00e22dfef..51d83b7557 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -611,14 +611,16 @@  static void update_options(char **dest, const char *name, void *src)
 static int open_url_keepalive(AVFormatContext *s, AVIOContext **pb,
                               const char *url)
 {
-    int ret;
+    int ret = 0;
     URLContext *uc = ffio_geturlcontext(*pb);
     av_assert0(uc);
     (*pb)->eof_reached = 0;
+#if CONFIG_HTTP_PROTOCOL
     ret = ff_http_do_new_request(uc, url);
     if (ret < 0) {
         ff_format_io_close(s, pb);
     }
+#endif
     return ret;
 }