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

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

> On Apr 24, 2025, at 11:08, Marvin Scholz <epirat07@gmail.com> wrote:
> 
> 
> 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.

Alright, fixed in v2.

https://ffmpeg.org/pipermail/ffmpeg-devel/2025-April/342706.html

> 
> (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...)

This doesn’t happen now since the code doesn't error out when read key failed.

I guess for key type SAMPLE_AES, audio track can be in clear, while the video track can be
partially encrypted.

There is a check at line 1408 which should be ret >= 0, but > 0 doesn’t happen in real.

1408     if (ret == 0 && !is_http && seg->url_offset) {


> 
>>                 }
>>                 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".

_______________________________________________
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".