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