Re: [FFmpeg-devel] [PATCH v3] avformat/hls: Return error code when read key file failed

[Zhao Zhili <quinkblack-at-foxmail.com@ffmpeg.org>]

> On Apr 29, 2025, at 20:15, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote:
> 
> Zhao Zhili:
>> From: Zhao Zhili <zhilizhao@tencent.com>
>> 
>> ---
>> libavformat/hls.c | 47 ++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 34 insertions(+), 13 deletions(-)
>> 
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index 6623c80309..fe908a563b 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -1331,6 +1331,37 @@ static void intercept_id3(struct playlist *pls, uint8_t *buf,
>>         pls->is_id3_timestamped = (pls->id3_mpegts_timestamp != AV_NOPTS_VALUE);
>> }
>> 
>> +static int read_key(HLSContext *c, struct playlist *pls, struct segment *seg)
>> +{
>> +    AVIOContext *pb = NULL;
>> +
>> +    int ret = open_url(pls->parent, &pb, seg->key, &c->avio_opts, NULL, NULL);
>> +    if (ret < 0) {
>> +        av_log(pls->parent, AV_LOG_ERROR, "Unable to open key file %s, %s\n",
>> +               seg->key, av_err2str(ret));
>> +        return ret;
>> +    }
>> +
>> +    ret = avio_read(pb, pls->key, sizeof(pls->key));
>> +    ff_format_io_close(pls->parent, &pb);
>> +    if (ret != sizeof(pls->key)) {
>> +        if (ret < 0) {
>> +            av_log(pls->parent, AV_LOG_ERROR, "Unable to read key file %s, %s\n",
>> +                   seg->key, av_err2str(ret));
>> +        } else {
>> +            av_log(pls->parent, AV_LOG_ERROR, "Unable to read key file %s, read bytes %d != %zu\n",
> 
> SIZE_SPECIFIER

Please elaborate.

> 
> Apart from that: Do we really need to two different log messages?

IO error vs invalid data can be important for this case. Any suggestions?

> 
>> +                   seg->key, ret, sizeof(pls->key));
>> +            ret = AVERROR(AVERROR_INVALIDDATA);
> 
> AVERROR() is wrong.

OK.

> 
>> +        }
>> +
>> +        return ret;
>> +    }
>> +
>> +    av_strlcpy(pls->key_url, seg->key, sizeof(pls->key_url));
>> +
>> +    return 0;
>> +}
>> +
>> static int open_input(HLSContext *c, struct playlist *pls, struct segment *seg, AVIOContext **in)
>> {
>>     AVDictionary *opts = NULL;
>> @@ -1352,19 +1383,9 @@ 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 = 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);
>> -                }
>> -                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_strlcpy(pls->key_url, seg->key, sizeof(pls->key_url));
>> +            ret = read_key(c, pls, seg);
>> +            if (ret < 0)
>> +                goto cleanup;
>>         }
>>     }
>> 
> 
> _______________________________________________
> 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".