| Message ID | 20171225034808.29895-1-lq@chinaffmpeg.org |
|---|---|
| State | New |
| Headers | show |
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 >
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.
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 >
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; }
fix --disable-network compipling error Signed-off-by: Steven Liu <lq@chinaffmpeg.org> --- libavformat/hls.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)