Paul B Mahol:
> On Wed, Sep 6, 2023 at 11:26 AM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>>
>>> + chunk_type = avio_rb32(pb);
>>> + chunk_size = avio_rb32(pb);
>>
>> You are not checking whether the chunk here exceeds its containing chunk.
>>
>>>
>>> + av_fast_malloc(&usm->header, &usm->header_size,
>>> + chunk_size + AV_INPUT_BUFFER_PADDING_SIZE);
>>> + if (!usm->header)
>>> + return AVERROR(ENOMEM);
>>
>> The bytestream2 API does not rely on the buffer being padded at all.
>>
>>>
>>> + bytestream2_skip(&sgb, string_offset);
>>
>> This is unnecessary, because you seek with an absolute offset lateron
>> anyway before using sgb.
>>
>>>
>>> + bytestream2_seek(&sgb, string_offset + offset, SEEK_SET);
>>> + while (bytestream2_get_bytes_left(&sgb) > 0) {
>>> + key[n] = bytestream2_get_byte(&sgb);
>>> + if (!key[n])
>>> + break;
>>> + if (n >= sizeof(key) - 1)
>>> + break;
>>> + n++;
>>> + }
>>> + key[n] = '\0';
>>
>> IMO this would be easier with strnlen(), avoiding sgb altogether.
>> You would of course need to explicitly check that you are not
>> overreading, but that is good practice anyway.
>>
>>>
>>> + chunk_start = avio_tell(pb);
>>> + avio_skip(pb, 1);
>>> + payload_offset = avio_r8(pb);
>>> + padding_size = avio_rb16(pb);
>>> + stream_index = avio_r8(pb);
>>> + avio_skip(pb, 2);
>>> + payload_type = avio_r8(pb);
>>> + frame_time = avio_rb32(pb);
>>> + frame_rate = avio_rb32(pb);
>>> + avio_skip(pb, 8);
>>
>> payload_offset and frame_time are set-but-unused; this might lead to
>> compiler warnings.
>>
>>> + if (usm->ch[is_audio][stream_index].used == 1) {
>>> + uint32_t pkt_size = chunk_size - (avio_tell(pb) -
>> chunk_start);
>>> +
>>
>> This is unnecessary: Unless we already had a read error, pkt_size is
>> chunk_size - (1 + 1 + 2 + 1 + 2 + 1 + 4 + 4 + 8).
>>
>> (Notice that in case padding_size is > 0, it will be part of the packet
>> with the current code; not sure if that is an issue.)
>>
>>>
>>> +
>>> + avio_skip(pb, padding_size);
>>> + avio_skip(pb, chunk_size - (avio_tell(pb) - chunk_start));
>>> +
>>
>> Simpler to just use avio_seek(pb, chunk_start + chunk_size, SEEK_SET);
>>
>
> But input might not be seekable.
>
And? You know that avio_skip(pb, offset) is just avio_seek(pb, offset,
SEEK_CUR)?
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
