Paul B Mahol:
> On Sun, Sep 10, 2023 at 2:06 PM Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
>> Paul B Mahol:
>>> On Sun, Sep 10, 2023 at 1:59 PM Andreas Rheinhardt <
>>> andreas.rheinha...@outlook.com> wrote:
>>>
>>>> Paul B Mahol:
>>>>> On Wed, Sep 6, 2023 at 1:30 PM Andreas Rheinhardt <
>>>>> andreas.rheinha...@outlook.com> wrote:
>>>>>
>>>>>> 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)?
>>>>>>
>>>>>
>>>>> And? Do you know that SEEK_SET is different from SEEK_CUR with positive
>>>>> argument.
>>>>>
>>>>
>>>> You are using SEEK_CUR with -avio_tell(pb), which effectively makes it
>>>> an absolute seek.
>>>>
>>>
>>> Nope, I skip data only, not seeking backward ever.
>>>
>>
>> avio_seek() internally translates any avio_seek() with SEEK_CUR to an
>> absolute seek (as required by the seek callbacks) and does not care
>> about whether it is seeking forward or backward.
>>
>
> Irrelevant. Seeking needs enough cache to work on non-seekable input.
>
Given that avio_skip() is just a wrapper around avio_seek(), the
behaviour will not change even when the input is non-seekable.
- 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".
