[FFmpeg-devel] [PATCH] avformat/scd: add demuxer
Zane van Iperen
zane at zanevaniperen.com
Tue Nov 23 09:05:03 EET 2021
On 23/11/21 16:18, Peter Ross wrote:
>>>> index cbfadcb639..1054ac9667 100644
>>>> --- a/libavformat/allformats.c
>>>> +++ b/libavformat/allformats.c
>>>> @@ -392,6 +392,7 @@ extern const AVOutputFormat ff_sbc_muxer;
>>>> extern const AVInputFormat ff_sbg_demuxer;
>>>> extern const AVInputFormat ff_scc_demuxer;
>>>> extern const AVOutputFormat ff_scc_muxer;
>>>> +extern const AVInputFormat ff_scd_demuxer;
>>>> extern const AVInputFormat ff_sdp_demuxer;
>>>> extern const AVInputFormat ff_sdr2_demuxer;
>>>> extern const AVInputFormat ff_sds_demuxer;
>
> the indentation here is inconsistent.
>
I think there may be something wrong with your viewer, all the indentation looks fine to me.
Does it look fine here? https://ffmpeg.org/pipermail/ffmpeg-devel/2021-November/287508.html
>>>> +static int scd_read_offsets(AVFormatContext *s)
>>>> +{
>>>> + int64_t ret;
>>>> + SCDDemuxContext *ctx = s->priv_data;
>>>> + uint8_t buf[SCD_OFFSET_HEADER_SIZE];
>>>> +
>>>> + if ((ret = avio_read(s->pb, buf, SCD_OFFSET_HEADER_SIZE)) < 0)
>>>> + return ret;
>>>> +
>>>> + ctx->hdr.table0.count = AV_RB16(buf + 0);
>>>> + ctx->hdr.table1.count = AV_RB16(buf + 2);
>>>> + ctx->hdr.table2.count = AV_RB16(buf + 4);
>>>> + ctx->hdr.unk2 = AV_RB16(buf + 6);
>>>> + ctx->hdr.table0.offset = AV_RB32(buf + 8);
>>>> + ctx->hdr.table1.offset = AV_RB32(buf + 12);
>>>> + ctx->hdr.table2.offset = AV_RB32(buf + 16);
>>>> + ctx->hdr.unk3 = AV_RB32(buf + 20);
>>>> + ctx->hdr.unk4 = AV_RB32(buf + 24);
>
> is there any reason why you read the values into buf?
>
> why not use avio_rb16/32(pb) to directly read the values. this is how other demuxers do it.
> also use avio_skip(pb, xxx) to skip over the unknown values.
> this saves having to define the structures and defining xxx_HEADER_SIZE.
>
#notalldemuxers - See argo_{asf,cvg,brp}, pp_bnk, kvag, alp, etc.
Jokes aside, I've always avoided avio_rbxx() because of the potential need for error handling.
I find it nicer to do one big read and AV_RBXX() it out, than avio_rbxx() each field.
As for structures and xxx_HEADER_SIZE - I find it's helpful for future documentation and
debugging purposes. It's nice to be able to look at the header structure when debugging,
for example.
The `buf + XX` also makes it clear what the field offsets are.
>>>> +
>>>> + track->length = AV_RB32(buf + 0);
>>>> + track->num_channels = AV_RB32(buf + 4);
>>>> + track->sample_rate = AV_RB32(buf + 8);
>>>> + track->data_type = AV_RB32(buf + 12);
>>>> + track->loop_start = AV_RB32(buf + 16);
>>>> + track->loop_end = AV_RB32(buf + 20);
>>>> + track->data_offset = AV_RB32(buf + 24);
>>>> + track->aux_count = AV_RB32(buf + 28);
>
> ditto
>
Ditto to your ditto.
More information about the ffmpeg-devel
mailing list