Re: [FFmpeg-devel] [PATCH 2/3] avformat/hls: Show error info when read key file failed

[Marvin Scholz <epirat07@gmail.com>]

On 23 Apr 2025, at 14:59, Zhao Zhili wrote:

> From: Zhao Zhili <zhilizhao@tencent.com>
>
> ---
>  libavformat/hls.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 6623c80309..45c1b283c9 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1353,16 +1353,18 @@ static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg,
>      if (seg->key_type == KEY_AES_128 || seg->key_type == KEY_SAMPLE_AES) {
>          if (strcmp(seg->key, pls->key_url)) {
>              AVIOContext *pb = NULL;
> -            if (open_url(pls->parent, &pb, seg->key, &c->avio_opts, NULL, NULL) == 0) {
> +
> +            ret = open_url(pls->parent, &pb, seg->key, &c->avio_opts, NULL, NULL);
> +            if (ret >= 0) {
>                  ret = avio_read(pb, pls->key, sizeof(pls->key));
>                  if (ret != sizeof(pls->key)) {
> -                    av_log(pls->parent, AV_LOG_ERROR, "Unable to read key file %s\n",
> -                           seg->key);
> +                    av_log(pls->parent, AV_LOG_ERROR, "Unable to read key file %s, %s\n",
> +                           seg->key, av_err2str(ret));

The check here is just checking if the size matches the expected size, lets say it just
reads 1 byte instead if 10, av_err2str(ret) called with 1 in this example wont give you
anything meaningful and even more unhelpful than the previous message.

You should check that ret is actually negative, else you should probably report that the file
ended unexpectedly, printing the expected size and amount that was actually read.

(Also unrelated to this patch but this ret is eventually returned by the function with a possibly
positive value in this case which seems wrong...)

>                  }
>                  ff_format_io_close(pls->parent, &pb);
>              } else {
> -                av_log(pls->parent, AV_LOG_ERROR, "Unable to open key file %s\n",
> -                       seg->key);
> +                av_log(pls->parent, AV_LOG_ERROR, "Unable to open key file %s, %s\n",
> +                       seg->key, av_err2str(ret));
>              }
>              av_strlcpy(pls->key_url, seg->key, sizeof(pls->key_url));
>          }
> -- 
> 2.46.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".