diff mbox

[FFmpeg-devel] avformat/hlsenc: Added context to av_log calls

Message ID 1511518350-26641-1-git-send-email-kjeyapal@akamai.com
State Superseded
Headers show

Commit Message

Jeyapal, Karthick Nov. 24, 2017, 10:12 a.m. UTC
---
 libavformat/hlsenc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comments

Liu Steven Nov. 24, 2017, 10:15 a.m. UTC | #1
> 在 2017年11月24日,18:12,Karthick J <kjeyapal@akamai.com> 写道:
> 
> ---
> libavformat/hlsenc.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 30ccf73..379a4ec 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1057,7 +1057,6 @@ static int get_relative_url(const char *master_url, const char *media_url,
>     if (p) {
>         base_len = FFABS(p - master_url);
>         if (av_strncasecmp(master_url, media_url, base_len)) {
> -            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");
>             return AVERROR(EINVAL);
>         }
>     }
> @@ -1096,7 +1095,7 @@ static int create_master_playlist(AVFormatContext *s,
>                      &options);
>     av_dict_free(&options);
>     if (ret < 0) {
> -        av_log(NULL, AV_LOG_ERROR, "Failed to open master play list file '%s'\n",
> +        av_log(s, AV_LOG_ERROR, "Failed to open master play list file '%s'\n",
>                 hls->master_m3u8_url);
>         goto fail;
>     }
> @@ -1118,7 +1117,7 @@ static int create_master_playlist(AVFormatContext *s,
>         ret = get_relative_url(hls->master_m3u8_url, vs->m3u8_name,
>                                m3u8_rel_name, m3u8_name_size);
>         if (ret < 0) {
> -            av_log(NULL, AV_LOG_ERROR, "Unable to find relative URL\n");
> +            av_log(s, AV_LOG_ERROR, "Unable to find relative URL\n");
>             goto fail;
>         }
> 
> @@ -1132,7 +1131,7 @@ static int create_master_playlist(AVFormatContext *s,
>         }
> 
>         if (!vid_st && !aud_st) {
> -            av_log(NULL, AV_LOG_WARNING, "Media stream not found\n");
> +            av_log(s, AV_LOG_WARNING, "Media stream not found\n");
>             continue;
>         }
> 
> @@ -1144,7 +1143,7 @@ static int create_master_playlist(AVFormatContext *s,
>         bandwidth += bandwidth / 10;
> 
>         if (!bandwidth) {
> -            av_log(NULL, AV_LOG_WARNING,
> +            av_log(s, AV_LOG_WARNING,
>                     "Bandwidth info not available, set audio and video bitrates\n");
>             av_freep(&m3u8_rel_name);
>             continue;
> -- 
> 1.9.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


LGTM


Thanks
Moritz Barsnick Nov. 24, 2017, 11:03 a.m. UTC | #2
On Fri, Nov 24, 2017 at 15:42:30 +0530, Karthick J wrote:
>          if (av_strncasecmp(master_url, media_url, base_len)) {
> -            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");
>              return AVERROR(EINVAL);

Was it intention to remove this one?

Moritz
Carl Eugen Hoyos Nov. 24, 2017, 11:05 a.m. UTC | #3
2017-11-24 12:03 GMT+01:00 Moritz Barsnick <barsnick@gmx.net>:
> On Fri, Nov 24, 2017 at 15:42:30 +0530, Karthick J wrote:
>>          if (av_strncasecmp(master_url, media_url, base_len)) {
>> -            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");
>>              return AVERROR(EINVAL);
>
> Was it intention to remove this one?

If yes, please mention in the commit message that this was
not a useful warning (or make it a separate commit).

Carl Eugen
Liu Steven Nov. 24, 2017, 11:21 a.m. UTC | #4
> 在 2017年11月24日,19:03,Moritz Barsnick <barsnick@gmx.net> 写道:
> 
> On Fri, Nov 24, 2017 at 15:42:30 +0530, Karthick J wrote:
>>         if (av_strncasecmp(master_url, media_url, base_len)) {
>> -            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");
>>             return AVERROR(EINVAL);
> 
> Was it intention to remove this one?
I think yes, because he move the av_log out of this call.
> 
> Moritz
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Jeyapal, Karthick Nov. 24, 2017, 11:31 a.m. UTC | #5
On 11/24/17, 4:36 PM, "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote:

>2017-11-24 12:03 GMT+01:00 Moritz Barsnick <barsnick@gmx.net>:

>> On Fri, Nov 24, 2017 at 15:42:30 +0530, Karthick J wrote:

>>>          if (av_strncasecmp(master_url, media_url, base_len)) {

>>> -            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");

>>>              return AVERROR(EINVAL);

>>

>> Was it intention to remove this one?

Thanks for that question.
Yes, it was removed intentionally, as the calling function was also printing the same log, with ERROR.
>

>If yes, please mention in the commit message that this was

>not a useful warning (or make it a separate commit).

I have amended the commit message and have sent a new patch v2.

Thanks and regards,
Karthick
diff mbox

Patch

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 30ccf73..379a4ec 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1057,7 +1057,6 @@  static int get_relative_url(const char *master_url, const char *media_url,
     if (p) {
         base_len = FFABS(p - master_url);
         if (av_strncasecmp(master_url, media_url, base_len)) {
-            av_log(NULL, AV_LOG_WARNING, "Unable to find relative url\n");
             return AVERROR(EINVAL);
         }
     }
@@ -1096,7 +1095,7 @@  static int create_master_playlist(AVFormatContext *s,
                      &options);
     av_dict_free(&options);
     if (ret < 0) {
-        av_log(NULL, AV_LOG_ERROR, "Failed to open master play list file '%s'\n",
+        av_log(s, AV_LOG_ERROR, "Failed to open master play list file '%s'\n",
                 hls->master_m3u8_url);
         goto fail;
     }
@@ -1118,7 +1117,7 @@  static int create_master_playlist(AVFormatContext *s,
         ret = get_relative_url(hls->master_m3u8_url, vs->m3u8_name,
                                m3u8_rel_name, m3u8_name_size);
         if (ret < 0) {
-            av_log(NULL, AV_LOG_ERROR, "Unable to find relative URL\n");
+            av_log(s, AV_LOG_ERROR, "Unable to find relative URL\n");
             goto fail;
         }
 
@@ -1132,7 +1131,7 @@  static int create_master_playlist(AVFormatContext *s,
         }
 
         if (!vid_st && !aud_st) {
-            av_log(NULL, AV_LOG_WARNING, "Media stream not found\n");
+            av_log(s, AV_LOG_WARNING, "Media stream not found\n");
             continue;
         }
 
@@ -1144,7 +1143,7 @@  static int create_master_playlist(AVFormatContext *s,
         bandwidth += bandwidth / 10;
 
         if (!bandwidth) {
-            av_log(NULL, AV_LOG_WARNING,
+            av_log(s, AV_LOG_WARNING,
                     "Bandwidth info not available, set audio and video bitrates\n");
             av_freep(&m3u8_rel_name);
             continue;