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

[Martin Storsjö]

On Thu, 24 Apr 2025, Zhao Zhili wrote:

>> On Apr 24, 2025, at 10:24, Steven Liu <lq at chinaffmpeg.org> wrote:
>>
>>
>>> On Apr 23, 2025, at 20:59, Zhao Zhili <quinkblack-at-foxmail.com at ffmpeg.org> wrote:
>> Hi Zhili,
>>>
>>> From: Zhao Zhili <zhilizhao at 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));
>>>                }
>>>                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));
>>>            }
>> Why not jump to cleanup immediately after an output error?
>
> Because I don't know why not return error when the code is added in 84465f21 since 2011, and not
> changed since then. I'm worried return error might trigger unknown bugs.
>
> Cc Martin. Is there some reason not to error out in this case? Does that reason still hold now?

I really don't remember if there was any specific reason for not wanting 
to error out directly at that point - I wouldn't think so.

So feel free to add that if it makes sense; it's most likely an oversight.

// Martin